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

[UI] Sidebars #13754

Draft
wants to merge 23 commits into
base: 5.x
Choose a base branch
from
Draft

[UI] Sidebars #13754

wants to merge 23 commits into from

Conversation

andersonjeccel
Copy link
Contributor

@andersonjeccel andersonjeccel commented May 15, 2024

Q A
Bug fix? (use the a.b branch) 🟢
New feature/enhancement? (use the a.x branch) 🟢
Deprecations? 🔴
BC breaks? (use the c.x branch) 🔴
Automated tests included? 🔴
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

This PR is based on another PR, where I introduce tokens to replace most of the UI variables, aiming to improve consistency in all components. We need it to be merged before this one becomes available for code review.

This PR:

  • Make things light to have a dark version
  • Adds dark theme
  • Fixes weird minimized sidebar size
  • Increases icon size when minimized

Before:

sidebars-before.mp4

After:

sidebars-after.mp4

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Interact with the sidebar

How to test dark mode?

  1. Open dev tools on your browser (F12)
  2. Open the Console tab
  3. On the field, paste document.documentElement.setAttribute('color-theme', 'dark'); and hit Enter
  4. The components changed by the specific PR you're seeing will become dark

@andersonjeccel andersonjeccel self-assigned this May 15, 2024
@andersonjeccel andersonjeccel requested review from a team, ricfreire and Esthertests May 15, 2024 17:57
@andersonjeccel andersonjeccel added bug Issues or PR's relating to bugs user-interface Anything related to appearance, layout, and interactivity ready-to-test PR's that are ready to test labels May 15, 2024
@andersonjeccel andersonjeccel added code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality T1 Low difficulty to fix (issue) or test (PR) labels May 15, 2024
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, understand the change, just minor for me

image

@andersonjeccel
Copy link
Contributor Author

@kuzmany I moved the Settings title to an accessible description for screen readers only

Why to have a title on the right sidebar if we don’t have a title on the left?

And the second sidebar is more than settings… How would a points group differ from a category? If one of them is a setting, the other also should be considered

perhaps we can be a bit more reasonable for users going this way, what do you think?

@kuzmany
Copy link
Member

kuzmany commented May 17, 2024

@andersonjeccel I've noticed the empty space. From a user's perspective, it appears to be a bug.I am not saying you need to fix it, just saying even if it fits all attributes, for basic users it could look like a bug in design.

Copy link
Contributor

@shinde-rahul shinde-rahul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the sidebar collapsed the menu items like,

  • Component
  • Channels
  • Points
    They are inaccessible. Please check this,
video1500268263.mp4

@andersonjeccel
Copy link
Contributor Author

@kuzmany I’m actually searching for perceptions like yours

what if we had a toggle for dark mode there? Would you find it a strange place to be?

my initial idea was to put the toggle on the account profile

@andersonjeccel
Copy link
Contributor Author

@shinde-rahul I’ll check it

Do you know if this functionality works normally on other instances?

@andersonjeccel
Copy link
Contributor Author

@shinde-rahul Checked, it seems like an old issue
I won't be able to pick it, probably needs technical effort only a dev could do

@andersonjeccel
Copy link
Contributor Author

How to test dark mode?

  1. Open dev tools on your browser (F12)
  2. Open the Console tab
  3. On the field, paste document.documentElement.setAttribute('color-theme', 'dark'); and hit Enter
  4. The components changed by the specific PR you're seeing will become dark

Copy link

@PatrickJenkner PatrickJenkner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in both light and dark mode 👍
I agree with @kuzmany that the empty space in the right sidebar looks weird. I like your idea of putting the toggle for dark mode there. That would make it more easily accessible than putting it in the profile

@andersonjeccel andersonjeccel marked this pull request as draft May 21, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs code-review-needed PR's that require a code review before merging enhancement Any improvement to an existing feature or functionality ready-to-test PR's that are ready to test T1 Low difficulty to fix (issue) or test (PR) user-interface Anything related to appearance, layout, and interactivity
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants