Page MenuHomePhabricator

[betalabs-regression] RC filters - UI changes in displaying conflict states and 'no-effect' filters
Closed, ResolvedPublic

Description

(1) Conflicting state

Currently in beta:

In production - plwiki:

(2) no-effect state

In production - plwiki:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 21 2017, 12:44 AM

We didn't change anything in the styling, but I think this might be due to the styling change that was done to the TagMultiselectWidget. @Volker_E, can confirm?

And if so, @Pginer-WMF do we want to override (it's possible, of course, I just want to verify) or leave the change as-is?

My quick thinking on this would be that we should leave the "no-effect" as-is (with the new ooui changes) but override the 'conflict' colors - since conflict is strictly an RCFilter-specific concept.

Just as an aside, the TagMultiselectWidget *does* have a styling for "invalid" tags (which is also red-border) but we are not using the "invalid" state because our tags aren't actually invalid, they just cause a conflict. That said, @Pginer-WMF let me know if you want the "conflict" state to be the same styling as the "invalid" state (in that case, I'll just toggle the oo-ui-tagItemWidget-invalid on the tag items when they're in conflict? It's a tiny cheat, but we gain, maybe, having the same design as the ooui one?)

In any case, @Pginer-WMF please let me know if you want to leave as-is, or override, and if you want to override *both* states or only the conflict state.

Regarding the no-effect state, it is important that tags in such state look less-emphasised than the regular ones but without looking disabled. In F7674854 the no-effect state is not recognisable. No-effect tags should look like in F7674803. This can be a specific customisation for the New Filters for Edit Review if we consider it too specific of this context.

Regarding the conflict state, I think it makes sense to use the same visual execution that is decided for invalid, even if they are not the same state both are flagging a problem with the tag and the difference (if screenshots F7674646 and T163522 shown both options) seem to be minimal if we wanted users to rely on them to distinguish from both states. I'll left up to you if semantically it makes sense to mark conflicts as "invalid", but I'd reuse the same visual style in any case.

@Mooeypoo @Pginer-WMF I've oriented on screens of RCFilters when improving the inputPosition:outline originally.
The concept of no-effect tags is new to me – I didn't understand the separation from the few screens only and thought they are disabled tags.
The “normal” tags have been using white as background-color as I thought one of the outcomes of our last UI Standardization meeting was, when framed elements are on dark backgrounds, they switch colors and receive white backgrounds. Referring to “Very likely good“ and “May have problems“ tags in F7674854. This helps for contrasting the interaction elements from the non-interactive surrounding.

Invalid tags have been defined in @Mooeypoo's original patch succeeding a short conversation. It seems, we all agree, that it should align to similar elements, invalid input widgets (compare: “NumberInputWidget (1–5, ints only)” in the demo) were the orientation point.
As some of this is unchartered territory, we have all options to improve the appearance.

Side note: I've updated the next steps of the TagMultiselect- and also in compatibility CapsuleMultiselectWidget improvements at T162965

I actually misunderstood; by "no-effect" filters, I thought we mean the "regular" state filters. I forgot about the actual "muted" / "no-effect".

To sum up what I'm going to do:

  • Have the "conflict" state styled the exact same as the "invalid" tag states for the OO.ui.TagItemWidget
  • Have the "no-effect" / "muted" state back to what it was before the OOUI change
  • Leave the 'normal' filter tags as-is adhering to the OOUI design.

Change 349507 had a related patch set uploaded (by Mooeypoo):
[mediawiki/core@master] RCFilters UI: Fix FilterTagItemWidget styles

https://gerrit.wikimedia.org/r/349507

@Mooeypoo Sounds good to me.

@Volker_E and @Pginer-WMF a couple of summarizing notes after working on this:

  • OO.ui.TagItemWidget is a FlaggableElement, so I just used the FlaggableElement methods (setFlag/etc) to set both 'invalid' and 'muted' states -- this means that now 'invalid' follows the same style as the base TagItemWidget in OOUI. However:
    • I overrode OOUI styles to make the label also red. The reason is, mostly, because when the tag is "selected" (and its border and background are blue in RCFilters) the 'conflict' state was hidden. It made sense to keep the label as red throughout -- but I think we should upstream it to OO.ui.TagItemWidget too. Invalid tags should also have their texts red, no?
    • OO.ui.TagItemWidget has a couple of styles that are way too specific and required me to create a rule that's 3 lines long. Not sure why that is, but perhaps an oversight? Can we try and unspecific-ise those? Specifically, I was stuck with background-color change (for selected tags) because of this rule:
.oo-ui-tagMultiselectWidget.oo-ui-widget-enabled.oo-ui-tagMultiselectWidget-outlined .oo-ui-tagMultiselectWidget-handle .oo-ui-tagItemWidget.oo-ui-widget-enabled {
   background-color: #fff;
}

Which, to override it, I had to write THIS rule:

.mw-rcfilters-ui-filterTagItemWidget {
	.oo-ui-tagMultiselectWidget.oo-ui-widget-enabled.oo-ui-tagMultiselectWidget-outlined .oo-ui-tagMultiselectWidget-handle &-selected.oo-ui-tagItemWidget.oo-ui-widget-enabled {
		background-color: #eaf3ff;
	}
}

...I don't know why the specificity is there. Why does the background need to be dependent on -outlined? can we please change this in OO.ui.TagItemWidget styles so I don't have to make insanely-long-selectors to override a simple background color?

  • I, uh, apparently, did not have the "base" mw-rcfilters-ui-filterTagItemWidget class actually being APPLIED to the tags. Oops. I fixed that.

@Volker_E and @Pginer-WMF a couple of summarizing notes after working on this:

  • OO.ui.TagItemWidget is a FlaggableElement, so I just used the FlaggableElement methods (setFlag/etc) to set both 'invalid' and 'muted' states -- this means that now 'invalid' follows the same style as the base TagItemWidget in OOUI. However:
    • I overrode OOUI styles to make the label also red. The reason is, mostly, because when the tag is "selected" (and its border and background are blue in RCFilters) the 'conflict' state was hidden. It made sense to keep the label as red throughout -- but I think we should upstream it to OO.ui.TagItemWidget too. Invalid tags should also have their texts red, no?

We can and probably should upstream this, the question remaining is for @Pginer-WMF and UX folks to decide if the invalid state is important to support in general. I'll prepare a small patch for now to change the color to Red50.

  • OO.ui.TagItemWidget has a couple of styles that are way too specific and required me to create a rule that's 3 lines long. Not sure why that is, but perhaps an oversight? Can we try and unspecific-ise those? Specifically, I was stuck with background-color change (for selected tags) because of this rule:
.oo-ui-tagMultiselectWidget.oo-ui-widget-enabled.oo-ui-tagMultiselectWidget-outlined .oo-ui-tagMultiselectWidget-handle .oo-ui-tagItemWidget.oo-ui-widget-enabled {
   background-color: #fff;
}

Which, to override it, I had to write THIS rule:

.mw-rcfilters-ui-filterTagItemWidget {
	.oo-ui-tagMultiselectWidget.oo-ui-widget-enabled.oo-ui-tagMultiselectWidget-outlined .oo-ui-tagMultiselectWidget-handle &-selected.oo-ui-tagItemWidget.oo-ui-widget-enabled {
		background-color: #eaf3ff;
	}
}

...I don't know why the specificity is there. Why does the background need to be dependent on -outlined? can we please change this in OO.ui.TagItemWidget styles so I don't have to make insanely-long-selectors to override a simple background color?

  • I, uh, apparently, did not have the "base" mw-rcfilters-ui-filterTagItemWidget class actually being APPLIED to the tags. Oops. I fixed that.

It's a complex combination of options. we need the -outlined selector as the tags on -inlined are having normal framed light grey #f8f9fa as they are on white background. The handle is probably unnecessary, it's just a leftover from the initial implementation. Will fix in that patch as well.

Another side-note, I appreciate the -enabled/-disabled selectors, but it took me some time to accept them, as instead of .oo-ui-widget-enabled I was hoping for .oo-ui-on/.oo-ui-off selectors. Verbosity takes getting used to.

Another side-note, I appreciate the -enabled/-disabled selectors, but it took me some time to accept them, as instead of .oo-ui-widget-enabled I was hoping for .oo-ui-on/.oo-ui-off selectors. Verbosity takes getting used to.

I don't mind what they're called (I understand enabled/disabled better than on/off which I think has a different semantic meaning, but do't have a strong preference) -- but the only reason they're actually in the style definition in RCFilters is for specificity :(

Change 349525 had a related patch set uploaded (by VolkerE):
[oojs/ui@master] MediaWiki theme: Decrease selector specificity and fix invalid appearance

https://gerrit.wikimedia.org/r/349525

Change 349525 merged by jenkins-bot:
[oojs/ui@master] MediaWiki theme: Decrease selector specificity and fix invalid appearance

https://gerrit.wikimedia.org/r/349525

Change 349507 merged by jenkins-bot:
[mediawiki/core@master] RCFilters UI: Fix FilterTagItemWidget styles

https://gerrit.wikimedia.org/r/349507

Current states in enwiki betalabs - @Mooeypoo, @Pginer-WMF please confirm that what it's should be:

Normal state


Conflicting state

No-effect state

Full coverage state

As per feedback form @Mooeypoo - normal, full-coverage, and conflicting state are displayed as expected.

'no-effect' state is filed as the separate issue: T163843: RC filters - 'no-effect' states displayed as not muted

QA Recommendation: Resolve

jmatazzoni closed this task as Resolved.Apr 26 2017, 1:43 AM