Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Feature] Add/Edit bookmark buttons in three-dot menu #17797

Closed
eliserichards opened this issue Feb 3, 2021 · 12 comments · Fixed by #18555
Closed

[Feature] Add/Edit bookmark buttons in three-dot menu #17797

eliserichards opened this issue Feb 3, 2021 · 12 comments · Fixed by #18555
Assignees
Labels
E5 Estimation Point: about 5 days eng:qa:verified QA Verified Feature:Bookmarks Feature:MainMenu The three-dot menu that is seen on the browser and homescreen. MR1 Issues that are needed for the MR1 2021 release. needs:ac Needs Android Component Work
Milestone

Comments

@eliserichards
Copy link
Contributor

eliserichards commented Feb 3, 2021

Meta: #17796
Followup to #17771

UX point of contact: @violasong (Victoria)

Figma designs (bottom right, labeled "Final design"): https://www.figma.com/file/NHu4cTmzfYgi3QJz5DIDWD/Fenix-Toolbar-Menus?node-id=0%3A1

Acceptance criteria

  • Create a button labeled "Add" that allows users to add the current page as a bookmark directly from the menu
  • Clicking on the ⭐ star should add the current tab as a bookmark
    • When the page is bookmarked, the button text should change to "Edit"
  • If the user is on a bookmarked page, clicking the ⭐ star again should remove the page from bookmarks without navigating away
  • The "Edit" button should navigate the user to the bookmark edit screen
  • The user should be able to navigate backwards from that screen

image image

┆Issue is synchronized with this Jira Task

@eliserichards eliserichards added Feature:Bookmarks Feature:MainMenu The three-dot menu that is seen on the browser and homescreen. labels Feb 3, 2021
@github-actions github-actions bot added the needs:triage Issue needs triage label Feb 3, 2021
@eliserichards eliserichards removed the needs:triage Issue needs triage label Feb 3, 2021
@eliserichards eliserichards added the MR1 Issues that are needed for the MR1 2021 release. label Feb 18, 2021
@eliserichards eliserichards added E3 Estimation Point: average, 2 - 3 days E5 Estimation Point: about 5 days and removed E3 Estimation Point: average, 2 - 3 days labels Feb 23, 2021
@mcarare mcarare added the needs:ac Needs Android Component Work label Mar 5, 2021
@mcarare mcarare self-assigned this Mar 8, 2021
@mcarare
Copy link
Contributor

mcarare commented Mar 8, 2021

Opened mozilla-mobile/android-components#9840 for the AC part.

@mcarare
Copy link
Contributor

mcarare commented Mar 18, 2021

@topotropic With the icons back in the menu, how will this item look? Wouldn't be redundant to have 2 star icons on the same row?

@mcarare mcarare added the needs:UX-feedback Needs UX Feedback label Mar 18, 2021
@violasong
Copy link
Collaborator

The updated look is in the figma file. This does end up with 2 star icons in the row, but I think the difference in icon styles makes this okay. CC: @vesta0 in case she has thoughts on this. If there's concern, I'll ask Bram to look into it.

image

mcarare added a commit to mcarare/fenix that referenced this issue Mar 19, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 19, 2021
@amedyne
Copy link
Contributor

amedyne commented Mar 22, 2021

@brampitoyo Any thoughts on the star icons for bookmarks?

@brampitoyo
Copy link

@amedyne I’m happy to go with @violasong’s recommendation up above, of using an additional star for the action of adding and editing a bookmark:

This does end up with 2 star icons in the row, but I think the difference in icon styles makes this okay.

@amedyne
Copy link
Contributor

amedyne commented Mar 23, 2021

Thanks for providing the feedback @brampitoyo. @mcarare See comment above.

@amedyne amedyne removed the needs:UX-feedback Needs UX Feedback label Mar 23, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 29, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 30, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 30, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 30, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 30, 2021
@gabrielluong gabrielluong added this to the 89 milestone Mar 30, 2021
mcarare added a commit to mcarare/fenix that referenced this issue Mar 31, 2021
mcarare added a commit that referenced this issue Mar 31, 2021
@cadeyrn
Copy link
Contributor

cadeyrn commented Mar 31, 2021

@mcarare Could the spacing between the star icon and the text increased a bit? The mockup shows more spacing than the current implementation. Thanks!

@mcarare
Copy link
Contributor

mcarare commented Mar 31, 2021

@violasong Can you please check the implementation and have it compared with the ones from Figma? And if there are any things that need to be changed, can you open an implementation review issue (like these implementation review )? Thank you!

LE: Can you also check on the dark theme colors? There were no specs for that in Figma.

@mcarare mcarare added the eng:qa:needed QA Needed label Mar 31, 2021
@eliserichards eliserichards linked a pull request Apr 5, 2021 that will close this issue
3 tasks
@lobontiumira
Copy link

Verified on the debug build with AC: 75.0.20210405143037, a69f74ece, and GV 89.0a1-20210405094633 on Google Pixel (Android 10), Samsung Galaxy Note 8 (Android 9), and HTC 10 (Android 8).
Tapping on the "Add" option from the page's three-dot menu, adds the page to Bookmarks.
Tapping on the "Edit" option redirects the user to the editing page of the bookmark.

However, the mockup shows more spacing than the current implementation.
@violasong could you please review?

bookmark.new.UI.mp4

bookmark

@lobontiumira lobontiumira added eng:qa:verified QA Verified and removed eng:qa:needed QA Needed labels Apr 6, 2021
@violasong
Copy link
Collaborator

Thanks for the screenshots!

Yes, there should be a 7px spacing between the star and the "Add/Edit" text - it does look like this is short some pixels. (FYI for the future, if you're signed into Figma you can use the Inspect mode to look at pixel measurements between elements)

The star icon should also be smaller - 19px instead of the standard 24px.

For dark mode, the purple text should be the main purple text color used in dark mode (Violet 40 - #ad3bff)

@eliserichards
Copy link
Contributor Author

I will open up a followup issue to tackle those. Thanks @violasong! 👍

@eliserichards
Copy link
Contributor Author

Followup filed: #18829

pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
E5 Estimation Point: about 5 days eng:qa:verified QA Verified Feature:Bookmarks Feature:MainMenu The three-dot menu that is seen on the browser and homescreen. MR1 Issues that are needed for the MR1 2021 release. needs:ac Needs Android Component Work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants