-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/sidebar refactor #75
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
The button refactor pr has been merged so you can update this pr as needed now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some changes needed for the buttons. Right now the button component isn't really being used and just ton of classes being added to it. I think we need a separate button component for those, like icon button in the figma, but it doesn't seem like it's in a completed state so I need to talk to Eury about this.
…x/chingu-dashboard into feature/sidebar-refactor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
Just some minor changes.
- Can you remove the cursor pointer on the myvoyage hover effect if the user is not part of active voyage or logged in. So basically when the lock appears.
- in the button component, in the neutral variant, change the focus to active.
- The tooltip became weird in your updated branch. EDIT: you can just move the tooltip to wrap around the button and not the element.icon. That fixes it.
@Dan-Y-Ko Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic to check for current voyage needs to change (again). Can just check for existence of an active status in the voyage status.
I tested with some method, you can use something else if it's better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm :)
Description
Refactor some UI problems related to the Sidebar after removing Daisy UI.
Important:
Buttons must be fixed, I will wait for the merge of the PR that refactor the Button component so I will use it to fix the buttons in the Sidebar.
Issue link
Fixes # (issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: