Closed Bug 1819352 Opened 1 year ago Closed 10 months ago

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

Categories

(Toolkit :: UI Widgets, task)

task

Tracking

()

RESOLVED FIXED
118 Branch
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)

Attached image 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.

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
  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.
  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!
Mentor: hjones
Keywords: good-first-bug
Whiteboard: [fidefe-reusable-components] → [fidefe-reusable-components][lang=js][lang=html][lang=css]

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

Flags: needinfo?(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.

Assignee: nobody → dvnishshanka
Status: NEW → ASSIGNED
Flags: needinfo?(hjones)

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.

Flags: needinfo?(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.

Flags: needinfo?(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!

Flags: needinfo?(dvnishshanka)

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

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.

Assignee: dvnishshanka → nobody
Status: ASSIGNED → NEW

Can I also work on this bug

Assignee: nobody → tosinorunesajo
Status: NEW → ASSIGNED
Attachment #9325565 - Attachment is obsolete: true
Flags: needinfo?(dvnishshanka)
Attachment #9325578 - Attachment description: Bug 1819352 -Use moz-button-group in report add-on dialog r=hjones → WIP: Bug 1819352 -Updated use moz-button-group in report add-on dialog
Attachment #9325578 - Attachment description: WIP: Bug 1819352 -Updated use moz-button-group in report add-on dialog → Bug 1819352 -Updated use moz-button-group in report add-on dialog r=hjones
Attachment #9325578 - Attachment description: Bug 1819352 -Updated use moz-button-group in report add-on dialog r=hjones → Bug 1819352 -corrected moz-button-group in report add-on dialog r=hjones
Attachment #9328713 - Attachment description: Bug 1819352 - corrected moz-button-group in report add-on dialog r=hjones → Bug 1819352 - corrected moz-button-group in report add-on dialog r=hjones,rpl
Attachment #9328713 - Attachment is obsolete: true

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

Flags: needinfo?(tosinorunesajo)

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

Assignee: tosinorunesajo → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(tosinorunesajo)

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

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: nobody → Shauryadubey123
Status: NEW → ASSIGNED

Depends on D182925

Attachment #9342617 - Attachment is obsolete: true
Attachment #9346386 - Attachment description: Bug 1819352 -Differential Revision: https://phabricator.services.mozilla.com/D182925 r?hjones → Bug 1819352 - fixed button padding in remote add-on dialogue to match convention r?hjones
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
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: