Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add separator to sidebar context menu between Show Sidebar and Position #27880

Closed
simonhong opened this issue Jan 18, 2023 · 1 comment · Fixed by brave/brave-core#16717
Closed

Add separator to sidebar context menu between Show Sidebar and Position #27880

simonhong opened this issue Jan 18, 2023 · 1 comment · Fixed by brave/brave-core#16717
Assignees
@simonhong
Labels
feature/sidebar Relating to Brave's Sidebar feature OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-macOS QA/Test-Plan-Specified QA/Yes release-notes/exclude
Milestone
1.48.x - Release

Comments

@simonhong
Copy link
Member

simonhong commented Jan 18, 2023 •
edited
Loading

F/u issue for #27160

Sidebar context menu should have separator above the Position like below

image

figma - https://www.figma.com/file/xOdKFSr3P5AXSAbP2r8ung/Sidebar-Desktop?node-id=3716%3A71686&t=yQuSi89twoq6zePW-0
@simonhong simonhong self-assigned this Jan 18, 2023
simonhong added a commit to brave/brave-core that referenced this issue Jan 18, 2023
@simonhong
Added separator to sidebar ctx menu between show sidebar and position
74ff8b8 
fix brave/brave-browser#27880

New separator type(BOTH_SIDE_PADDED_SEPARATOR) is introduced to have
padding on both side.
@simonhong simonhong mentioned this issue Jan 18, 2023
Merged
25 tasks
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Jan 19, 2023
@brave-builds brave-builds added this to the 1.49.x - Nightly milestone Jan 19, 2023
@brave-builds brave-builds mentioned this issue Jan 19, 2023
Merged
7 tasks
@stephendonner
Copy link

Verified PASSED using

Brave 1.48.139 Chromium: 109.0.5414.87 (Official Build) beta (x86_64)
Revision 2dc18eb511c56e012081b4abc9e38c81c885f7d4-refs/branch-heads/5414@{#1241}
OS macOS Version 11.7.2 (Build 20G1020)

Steps:

  1. installed 1.48.139
  2. launched Brave
  3. context-clicked on the sidebar
  4. noted the added separator ___________ line between the Show sidebar and Position sections
Sidebar left Sidebar right
Screen Shot 2023-01-24 at 10 37 34 PM Screen Shot 2023-01-24 at 10 38 19 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@simonhong simonhong

Labels
feature/sidebar Relating to Brave's Sidebar feature OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-macOS QA/Test-Plan-Specified QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.48.x - Release
Development

Successfully merging a pull request may close this issue.

Added separator to sidebar ctx menu between show sidebar and position brave/brave-core
4 participants
@stephendonner @rebron @simonhong @brave-builds