1819352 - Use moz-button-group in report add-on dialog

Status: RESOLVED FIXED

(Toolkit :: XUL Widgets, task)

Description

[Mark Striemer [:mstriemer]]

Attachment: image.png

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.

Comment 1

[Hanna Jones [:hjones]]

If you're an Outreachy applicant or external contributor wanting to pick up this bug, please follow these steps:

1. Comment here on the bug that you want to volunteer to help. This will tell others that you're working on the next steps.
2. 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.
3. 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 the moz-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.
4. 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 the browser_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 using mach lint
5. 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.
6. 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

[dvnishshanka]

Hi everyone. I am an Outreachy applicant, I would like to work on this bug.

Comment 3

[Hanna Jones [:hjones]]

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

[dvnishshanka]

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

[Hanna Jones [:hjones]]

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

[Hanna Jones [:hjones]]

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 7

[tosinsajo]

Since there’s no response can I work on this bug?

Comment 8

[Hanna Jones [:hjones]]

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 9

[Jennifer]

Can I also work on this bug

Comment 10

[tosinsajo]

Attachment: Bug 1819352 -Use moz-button-group in report add on dialog. r=hjones

Comment 11

[tosinsajo]

Attachment: Bug 1819352 -corrected moz-button-group in report add-on dialog r=hjones

Comment 12

[tosinsajo]

Attachment: Bug 1819352 - corrected moz-button-group in report add-on dialog r=hjones,rpl

Depends on D173909

Comment 13

[tosinsajo]

Attachment: Bug 1819352 - corrected moz-button-group in report add-on dialog r=hjones,rpl

Depends on D175567

Comment 14

[Hanna Jones [:hjones]]

hey :tosinsajo are you still planning on working on this?

Comment 15

[Mark Striemer [:mstriemer]]

Feel free to update your patch or let us know if you're free to take this on again, thanks!

Comment 16

[Shauryadubey123]

Hi there, first time contributor, I would like to work on this issue. Thank you

Comment 17

[Hanna Jones [:hjones]]

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.

Comment 18

[Shauryadubey123]

Attachment: Bug 1819352 - fixed button padding in remote add-on dialogue to match convention r?hjones

Comment 19

[Shauryadubey123]

Attachment: Bug 1819352 - removing commented css code, and redundant flexbox styling r?hjones

Depends on D182925

Comment 20

[Shauryadubey123]

Attachment: Bug 1819352 - fixed button padding in remote add-on dialogue to match convention r?hjones

Comment 21

[Pulsebot]

Pushed by [e-mail]:
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

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c910e4c0560e