| MMiller_WMF | |
| Oct 11 2021, 2:33 AM |
| F34711842: Kapture 2021-10-26 at 13.56.32.webm | |
| Oct 26 2021, 11:59 AM |
| F34710455: Screenshot 2021-10-25 at 18.08.16.png | |
| Oct 25 2021, 4:17 PM |
| F34710190: image.png | |
| Oct 25 2021, 12:08 PM |
| F34710184: image.png | |
| Oct 25 2021, 12:08 PM |
| F34710187: image.png | |
| Oct 25 2021, 12:08 PM |
| F34710085: image.png | |
| Oct 25 2021, 12:08 PM |
| F34693733: Progressive help cta.jpg | |
| Oct 18 2021, 9:49 AM |
| F34683467: image.png | |
| Oct 11 2021, 2:33 AM |
As part of the work for "add an image", we are going to change the style of the help panel button on both desktop and mobile, for all situations where it appears (whether inside a structured task or not).
Instead of being blue with a white question mark...
...it's going to be white with a blue question mark (and blue outline):
Figma: https://www.figma.com/file/ULhJr1isDstRbGE5vjYDsr/Add-images-structured-task?node-id=3042%3A8783
| Project | Branch | Lines +/- | Subject | |
|---|---|---|---|---|
| mediawiki/extensions/GrowthExperiments | master | +175 -79 | Help panel: lighter styling for help button |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | MMiller_WMF | T285587 [EPIC] "Add an image" Iteration 1 | |||
| Resolved | DMburugu | T289823 Add an image: frontend tasks for initial release | |||
| Resolved | Sgs | T292933 Help panel: lighter styling for help panel button |
Hi @Sgs I'm looking at T293147: Structured Tasks: Change how the inspector is collapsed on mobile where the help button will be present as well (but the mechanism for attaching it to the document & positioning it will be different). Perhaps it would make sense to make the help panel button a class (instead of having it be inline in HelpPanel.cta.js) so that ext.growthExperiments.StructuredTask can also include the same class as a dependency.and use the same element or perhaps the CSS could be shared via a mixin.
@mewoph it totally makes sense. I will start taking the ButtonWidget code from HelpPanel to a separate component.
@RHo there's a slight divergence between the Figma designs and standard OOUI states (active and focus). Very slight difference in the icon color (active and focus) and the border/outline (active). We can add extra styles to match the designs. For the focus border I think it's worth since makes the tab navigation coherent.
@RHo there's a slight divergence between the Figma designs and standard OOUI states (active and focus). Very slight difference in the icon color (active and focus) and the border/outline (active). We can add extra styles to match the designs. For the focus border I think it's worth since makes the tab navigation coherent.
Hi @Sgs, I agree that it would be better to add the extra styles in the design so that the box-shadow is kept for the "floating" style on focus. Thanks for the detailed review and clear recommendation!
@mewoph I extracted the help cta style and instantiation to a separate component (See 731349). I would need your input to clarify what to do with the getHelpPanelCtaButton in HelpPanel.php:30 since it also holds some button properties.
Also we have several points where mw-ge-help-panel-cta-button CSS id is set or used. Maybe we could consolidate some of these. ie modules/helppanel/ext.growthExperiments.HelpPanel.cta.js:36
Hi @Sgs, for the button from getHelpPanelCtaButton, unfortunately the attributes would have to be kept in sync manually with the new component (we have several of this pattern where PHP & JS attributes need to align). Right now when the button is returned in the initial HTML, it's infused by OO.ui.ButtonWidget so I think instead this can be infused by the new HelpPanelButton class.
For consolation, do you mean creating a variable for the ID? I think the duplication may have been there because doing $(selectorInVariable) would trigger MediaWiki's lint rule to spell out the selector name in full.
When this change will be ready to be deployed, I think we should announce it to communities through Tech News. It is small, but it is a good way to remind our communities that these features exist.
Thanks for the insights @mewoph
I pushed another patch using infuse method from the HelpPanelButton class. See https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/731349
For consolation, do you mean creating a variable for the ID? I think the duplication may have been there because doing $(selectorInVariable) would trigger MediaWiki's lint rule to spell out the selector name in full.
Yes that's what I mean. Alright better to wait for other ways of solving this problem (ie: Vue adoption)
Change 731349 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):
[mediawiki/extensions/GrowthExperiments@master] Help panel: lighter styling for help button
I'm reluctant to include an entry for a minor design tweak, if it's just for the side-benefit of reminding users about ongoing feature development. I will leave it out for now.
Test wiki created on Patch Demo by SGimeno (WMF) using patch(es) linked to this task:
Test wiki created on Patch Demo by SGimeno (WMF) using patch(es) linked to this task:
@RHo I have created a patchdemo instance (https://patchdemo.wmflabs.org/wikis/f7c0c93022/w/) so the updated styles can be confirmed.
Hi @Sgs - thanks for spinning this up for review. A couple of things:
i. the Desktop version of the help panel is different from expected on the selected and selected focus states:
Expected styling for selected (with "?" turning into the downwards chevron  | Actual  |
Having said that, I am fine with this as a button instead of a toggle button if it will add substantial work to change the code.
ii. Help icon has an unexpected opacity:.87 instead of 1 across the different states:
Test wiki created on Patch Demo by SGimeno (WMF) using patch(es) linked to this task:
Thanks for the feedback @RHo, I pushed another patch to address both issues. You can check the demo here
Hi @Sgs - thanks for spinning this up for review. A couple of things:
i. the Desktop version of the help panel is different from expected on the selected and selected focus states:
Expected styling for selected (with "?" turning into the downwards chevron Actual Having said that, I am fine with this as a button instead of a toggle button if it will add substantial work to change the code.
The current implementation is using a standard ButtonWidget from OOUI not the ToggleButtonWidget so we have to handle the toggled state manually. Could be improved in the future but for now I think the behavior mimics well the toggle button.
Question: Where does the question mark with blue background state apply in our case?
Test wiki created on Patch Demo by SGimeno (WMF) using patch(es) linked to this task:
Thanks for the feedback @RHo, I pushed another patch to address both issues. You can check the demo here
Hi @Sgs - thanks for spinning this up for review. A couple of things:
i. the Desktop version of the help panel is different from expected on the selected and selected focus states:
Expected styling for selected (with "?" turning into the downwards chevron Actual Having said that, I am fine with this as a button instead of a toggle button if it will add substantial work to change the code.
The current implementation is using a standard ButtonWidget from OOUI not the ToggleButtonWidget so we have to handle the toggled state manually. Could be improved in the future but for now I think the behavior mimics well the toggle button.
Question: Where does the question mark with blue background state apply in our case?
Oh I included it in there as the beginning of the transition to the downwards chevron icon but actually you would never see it in our case. Sorry for that confusion, I can remove it from the figma!
Change 731349 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Help panel: lighter styling for help button
Test wiki on Patch Demo by SGimeno (WMF) using patch(es) linked to this task was deleted:
Test wiki on Patch Demo by SGimeno (WMF) using patch(es) linked to this task was deleted:
Test wiki on Patch Demo by SGimeno (WMF) using patch(es) linked to this task was deleted:
Test wiki on Patch Demo by SGimeno (WMF) using patch(es) linked to this task was deleted: