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.

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

Fix: Move settings from bottom to the top in the sidebar #931

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

CharliePops
Copy link
Collaborator

@CharliePops CharliePops commented Jan 30, 2025

Description

Move settings from bottom to the top in the sidebar

Relevant Technical Choices

Remove spacer added to the previous item in the sidebar

Testing Instructions

  1. Pull the branch fix/setting-menu-item
  2. Build the project npm run build:all
  3. Navigate to example.com
  4. Open privacy sandbox tab on DevTools
  5. Observer: the "Settings" sidebar item is under the "Wiki" item at the top, not in the bottom

Screenshot/Screencast

  • Before:
Screenshot 2025-01-29 at 10 25 09 PM
  • After:
Screenshot 2025-01-29 at 10 23 48 PM

Checklist

  • I have thoroughly tested this code to the best of my abilities.
  • I have reviewed the code myself before requesting a review.
  • NA [ ] This code is covered by unit tests to verify that it works as intended.
  • The QA of this PR is done by a member of the QA team (to be checked by QA).

@CharliePops CharliePops changed the title remove space between setting bar item and previous item Fix: Move settings from bottom to the top in the sidebar Jan 30, 2025
@mohdsayed mohdsayed requested a review from mayan-000 January 30, 2025 06:45
@mohdsayed
Copy link
Collaborator

@CharliePops Please do not skip the Checklist. At least the first two can always be checked.

@mohdsayed mohdsayed added this to the v0.13.0 milestone Jan 30, 2025
@mohdsayed
Copy link
Collaborator

@CharliePops Should we also move the settings icon to the same place in the collapsed mode?

CleanShot 2025-01-30 at 17 15 18

@CharliePops
Copy link
Collaborator Author

CharliePops commented Jan 30, 2025

@mohdsayed yeah that totally makes sense, I just moved it up:

Screenshot 2025-01-30 at 4 19 09 PM Screenshot 2025-01-30 at 4 19 45 PM

Selected and not selected states

@CharliePops CharliePops force-pushed the fix/setting-menu-item branch from 7cf2bae to 1044470 Compare January 30, 2025 22:14
@mohdsayed mohdsayed merged commit 8781a0d into develop Feb 3, 2025
6 checks passed
@mohdsayed mohdsayed deleted the fix/setting-menu-item branch February 3, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants