Use moz-button-group in report add-on dialog
Categories
(Toolkit :: XUL Widgets, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox118 | --- | fixed |
People
(Reporter: mstriemer, Assigned: Shauryadubey123, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: good-first-bug, Whiteboard: [fidefe-reusable-components][lang=js][lang=html][lang=css])
Attachments
(5 files, 3 obsolete files)
The Cancel/Go back and Next/Submit buttons in the report add-on dialog don't follow our conventions for what buttons should look like in dialogs like this.
There are some custom styles to make the buttons shorter and wider that can be removed (the rule and associated --button-padding
variable) [1]. And the container of the buttons should be updated to use the moz-button-group
component so that they're correctly positioned on all platforms.
Updated•7 months ago
|
Comment 1•7 months ago
|
||
If you're an Outreachy applicant or external contributor wanting to pick up this bug, please follow these steps:
- Comment here on the bug that you want to volunteer to help. This will tell others that you're working on the next steps.
- Download and build the Firefox source code
- If you have any problems, please ask on Element/Matrix in the
#introduction
channel. They're there to help you get started. - You can also read the Firefox Contributors' Quick Reference, which has answers to most development questions.
- If you have any problems, please ask on Element/Matrix in the
- Start working on this bug.
- To find this dialog in Firefox, navigate to
about:addons
and add at least one addon (if you don't already have any installed). Find an addon in the "Enabled" section, click on the "..." menu, then click on "Report" to bring up the dialog. - The code For the buttons can be found here.
- You'll need to wrap both sets of buttons with a
moz-button-group
, then double check to see if there's any CSS that needs to be removed (anything that changes the size or spacing of the buttons). You may need to add a script tag to make themoz-button-group
element available in the dialog. You can see an example of similar work in Bug 1802377. - If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the
#reusable-components
channel on Element/Matrix most hours of most days.
- To find this dialog in Firefox, navigate to
- Build your change with
mach build
and verify your changes locally. You can also test your change by running some of the abuse report tests (in particular thebrowser_html_abuse_report.js
tests). More information on running tests can be found here. Also check your changes for adherence to our style guidelines by usingmach lint
- Submit the patch (including an automated test, if applicable) for review. Mark me as a reviewer so I'll get an email to come look at your code.
- Getting your code reviewed
- This is when the bug will be assigned to you.
- After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!
Comment 2•7 months ago
|
||
Hi everyone. I am an Outreachy applicant, I would like to work on this bug.
Comment 3•7 months ago
|
||
Great, thanks! I've gone ahead and set you as the assignee for this bug. You should be able to get set up and started on the bug by following the steps in my comment above. If you run into any issues or have questions you can request information from me using the form below, or reach out to me and my teammates in #reusable-components
on Matrix.
Comment 4•7 months ago
|
||
Hi,
This is how it looks when I fixed the bug.
before: https://drive.google.com/file/d/1vBW0g7vJPMdTNF5lH_CqFja3OaFQI1bd/view?usp=share_link
after: https://drive.google.com/file/d/155Jy-FpLjCO7Cs24P2OVuyumrUqk_rwE/view?usp=share_link
I changed the immediate 'div' of buttons to 'moz-button-group'. Is there anything that need to change here.
Comment 5•7 months ago
|
||
That definitely looks like you're on the right track, thanks! I think there might be some bottom margin/padding that got removed which we'll still want - it can be added to the moz-button-group
element to preserve the same spacing around the elements.
At this point the best next step would be to submit your code from review and we can continue this conversation in the form of code comments. You can find information on submitting your code for review here.
Let me know if you have any other questions.
Comment 6•7 months ago
|
||
Hey :dvnishshanka - have you been able to commit your work? When it's ready for review you can run hg commit -m Bug 1819352 - <your commit message> r=hjones
, then run moz-phab submit
. Let me know if you're having issues or have any questions!
Comment 8•6 months ago
|
||
Hey :tosinsajo I've unassigned the bug if you want to work on it. There are instructions for getting started in this comment. Feel free to submit a patch, the bug will be assigned to you at that point.
Comment 10•6 months ago
|
||
Updated•6 months ago
|
Comment 11•6 months ago
|
||
Updated•6 months ago
|
Reporter | ||
Updated•6 months ago
|
Updated•6 months ago
|
Updated•6 months ago
|
Updated•6 months ago
|
Comment 12•6 months ago
|
||
Depends on D173909
Updated•6 months ago
|
Comment 13•6 months ago
|
||
Depends on D175567
Updated•6 months ago
|
Comment 14•4 months ago
|
||
hey :tosinsajo are you still planning on working on this?
Reporter | ||
Comment 15•3 months ago
|
||
Feel free to update your patch or let us know if you're free to take this on again, thanks!
Assignee | ||
Comment 16•3 months ago
|
||
Hi there, first time contributor, I would like to work on this issue. Thank you
Comment 17•3 months ago
|
||
hey :Shauryadubey123 that's great, thanks! Please take a look at the work that was already done in this patch as well as the comment about adding a test that was never incorporated - that should give you a good idea of what you need to do. Feel free to reach out if you have any questions.
Assignee | ||
Comment 18•3 months ago
|
||
Updated•3 months ago
|
Assignee | ||
Comment 19•3 months ago
|
||
Depends on D182925
Updated•3 months ago
|
Assignee | ||
Comment 20•2 months ago
|
||
Updated•2 months ago
|
Comment 21•2 months ago
|
||
Pushed by hjones@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c910e4c0560e fixed button padding in remote add-on dialogue to match convention r=hjones,extension-reviewers,desktop-theme-reviewers,dao,robwu
Comment 22•2 months ago
|
||
bugherder |
Description
•