1355170 - the 'rank' field label is incorrectly visible in view mode again

Status: NEW

(bugzilla.mozilla.org :: User Interface, defect, P3)

Description

[Matthew N. [:MattN]]

It looks like bug 1262474 has regressed as I'm seeing the label again in view mode.

+++ This bug was initially created as a clone of Bug #1262474 +++

if you're a member of a group that can set the rank field, but there isn't a value set, the label of the rank field is incorrectly visible in view mode.

the rank label should only be visible in view mode when a rank value has been set.

Comment 1

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

I want to think about that a minute. 

I think it's okay to show the rank field if it's not been set to socialize that the field exists.

Resolving as WORKSFORME, but willing to reconsider.

I think we should also rethink who has access to that field, but that's another bug.

Comment 2

[Matthew N. [:MattN]]

I disagree for many reasons:
1) Nothing relevant has changed since bug 1262474 was fixed AFAIK.
2) We hide other non-essential fields in view mode when they're unset as this is no different.
3) It looks bad with a label floating in the middle of nowhere since the default value is an empty string. There needs to at least be "---" or some other visual clue that the field is unset rather than looking like a UI glitch.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Can you point me at an example bug where this is the case? Nothing should have changed this, so I imagine it's a simple fix.

Comment 4

[Matthew N. [:MattN]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1350264
https://cl.ly/3A0j281Z0106

Comment 5

[Dylan Hardison [:dylan] (he/him)]

(In reply to Emma Humphries ☕️ (she/her) [:emceeaich] (UTC-8) +needinfo me from comment #1)
> I want to think about that a minute. 
> 
> I think it's okay to show the rank field if it's not been set to socialize
> that the field exists.
> 
> Resolving as WORKSFORME, but willing to reconsider.
> 
This isn't a good WORKSFORME because we don't know *why* this changed. It's fine if we later make this as a conscious decision, but if things change unintentionally, we have to investigate why.

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Looks like this regressed in bug 1283012

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Looks like we have: <div class="field inline edit-show" id="field-cf_rank" style="display:none">..
and the display: none is ignored because of a display: table-cell !important set for all .inline elements.

Not using !important for .inline causes a number of problems... I guess adding !important to the style="" element may work.

Comment 8

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

If we're using !important in our style definitions, that that's a definite code smell we ought to address. Agree with reopening this.

Comment 9

[Kohei Yoshino [:kohei]]

Will take a look after dropping tables from the modal UI.