Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | kostajh | T337566 [EPIC]: Incident Reporting System - Minimal Testable Product (MTP) | |||
| Resolved | • eigyan | T344906 Fix discrepancies between existing code and design specifications | |||
| Resolved | kostajh | T344909 Step 1 of form should have note about sending report to admin in the footer |
Event Timeline
Change 955724 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[mediawiki/extensions/ReportIncident@master] form: Add border above footer
With the patch:
| Desktop step one |  |
| Desktop step two |  |
| Mobile step one |  |
| Mobile step two |  |
@JSengupta-WMF @Madalina two questions:
- The designs show a border for mobile, but not for desktop. Is that deliberate? From a code perspective, it's easier to have a single rule that sets the border above the footer for both mobile and desktop.
- The designs show the "Your report will be sent to an admin" text below the footer border. It will be less code to maintain if we keep this text above the footer, where it currently is in the screenshots. That also has the added benefit of keeping the footer size consistent as the user navigates from step one to step two. What do you think?
@kostajh just jumping in here hope that's alright!
re 1. I used only Codex components and sometimes the rules for a component differ between desktop and mobile. The desktop dialog component's footer text doesn't have a divider but mobile does. I suggest we use components Codex (if you're not already) and follow their guidelines since that'll create the greatest amount of consistency across our designs. So yes, the border for mobile but not for desktop is deliberate.
re 2. I don't think we should change the design on the fly. The footer text is a property of the dialog component and the DS team has placed it where it is for a reason. Footer text isn't meant to be "content" in the dialog but is a foot note and this particular bit of text ("Your report will be sent to an Admin) is a foot note because we want the user to see it before proceeding in the process and clicking one of the buttons.
ofc this is now Joydeep's call so I'll let him chime in!
Of course! Thank you for doing so :)
re 1. I used only Codex components and sometimes the rules for a component differ between desktop and mobile. The desktop dialog component's footer text doesn't have a divider but mobile does. I suggest we use components Codex (if you're not already) and follow their guidelines since that'll create the greatest amount of consistency across our designs. So yes, the border for mobile but not for desktop is deliberate.
We are indeed using Codex components. The reason the border thing is coming up is because I don't see the border as built-in behavior to the dialog component, at least not on https://doc.wikimedia.org/codex/latest/components/demos/dialog.html or in practice, when we use this in MediaWiki (or visiting the demo site with a mobile device or emulator). Could you please point me towards where you're seeing a built-in border for mobile dialogs? Is it possible it's a difference between the Figma spec but not in the production Codex code?
re 2. I don't think we should change the design on the fly. The footer text is a property of the dialog component and the DS team has placed it where it is for a reason. Footer text isn't meant to be "content" in the dialog but is a foot note and this particular bit of text ("Your report will be sent to an Admin) is a foot note because we want the user to see it before proceeding in the process and clicking one of the buttons.
OK, thanks. The existing code wasn't using the footer-text property but I'll try using that.
Agree with Aishwarya regarding the Codex component. If we are already using them, it shouldn't introduce extra code to maintain if I understood correctly?
regarding the Codex component. If we are already using them, it shouldn't introduce extra code to maintain if I understood correctly?
OK, I think I see the point of disagreement/confusion. Codex does indeed provide a border on mobile, but only in cases where the form contents exceed the height of the viewport. On my desktop computer, I didn't see the overflow, and thus didn't see the border. I am not sure about Figma, but presumably it always shows you that border, even though in practice, the border only displays in an overflow scenario.
| No overflow |  |
| Overflow |  |
So, I believe the status quo (border appears above footer in mobile only when the content overflows the viewport) meets design expectations, but please let me know if not.
As for the positioning of the "Your report will be sent to an admin to review" text, we should be able to place it in the footer and hide it on step 2. Using footer-text is not compatible with using footer, so I couldn't make use of that template. And since we aren't using standard actions in the dialog, we need a custom footer template.
Change 955724 abandoned by Kosta Harlan:
[mediawiki/extensions/ReportIncident@master] form: Set border above footer
Reason:
See T344909#9177754
Change 958902 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[mediawiki/extensions/ReportIncident@master] form: Render help text in footer of step 1
Change 958902 merged by jenkins-bot:
[mediawiki/extensions/ReportIncident@master] form: Render help text in footer of step 1