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

refactor: sidebar #526

Merged
merged 16 commits into from
Sep 9, 2024
Merged

refactor: sidebar #526

merged 16 commits into from
Sep 9, 2024

Conversation

MarioRodrigues10
Copy link
Member

Description

The purpose of this PR is to refactor the sidebar, since it was using alpine and other outdated stuff.

Related Issues

N/A

Steps to reproduce or test

  • mix phx.server
  • Open at localhost:4000
  • And see the sidebar in desktop and mobile views

@MarioRodrigues10 MarioRodrigues10 self-assigned this Aug 22, 2024
@MarioRodrigues10 MarioRodrigues10 force-pushed the mr/update-sidebar branch 2 times, most recently from 13d40d8 to ee44bbc Compare August 22, 2024 18:26
ruilopesm
ruilopesm reviewed Aug 22, 2024
View reviewed changes
ruilopesm
ruilopesm reviewed Aug 22, 2024
View reviewed changes
Copy link
Member

@ruilopesm ruilopesm left a comment

Choose a reason for hiding this comment

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

Nice refactor. Just check my review and it's ready to merge.

Btw, I'm thinking that we need to find a better way to organize our components under the components folder. It's not a big deal, but it's starting to be a bit messy 😅

joaodiaslobo
joaodiaslobo requested changes Sep 1, 2024
View reviewed changes
Copy link
Member

@joaodiaslobo joaodiaslobo left a comment

Choose a reason for hiding this comment

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

Why is the user dropdown not aligned to the bottom anymore? Is this intended?

image

joaodiaslobo
joaodiaslobo requested changes Sep 1, 2024
View reviewed changes
Copy link
Member

@joaodiaslobo joaodiaslobo left a comment

Choose a reason for hiding this comment

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

Also, make sure that when the user clicks on the dark area on mobile, the sidebar closes.

@MarioRodrigues10
Copy link
Member Author

Thanks @joaodiaslobo 🙌

I removed a lot of bloat from div classes and somehow i moved the user up in the sidebar, I'll get that fixed today

@joaodiaslobo joaodiaslobo self-requested a review September 9, 2024 17:16
joaodiaslobo
joaodiaslobo approved these changes Sep 9, 2024
View reviewed changes
Copy link
Member

@joaodiaslobo joaodiaslobo left a comment

Choose a reason for hiding this comment

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

Nice refactor, thanks!

@joaodiaslobo joaodiaslobo merged commit b47933b into develop Sep 9, 2024
2 checks passed
@joaodiaslobo joaodiaslobo deleted the mr/update-sidebar branch September 9, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joaodiaslobo joaodiaslobo joaodiaslobo approved these changes

@nunom27 nunom27 Awaiting requested review from nunom27

@rfaria107 rfaria107 Awaiting requested review from rfaria107

@FilipeR13 FilipeR13 Awaiting requested review from FilipeR13

@JoaoCoelho2003 JoaoCoelho2003 Awaiting requested review from JoaoCoelho2003

@AfonsoMartins26 AfonsoMartins26 Awaiting requested review from AfonsoMartins26

@faria-s faria-s Awaiting requested review from faria-s

@ruilopesm ruilopesm Awaiting requested review from ruilopesm

Assignees

@MarioRodrigues10 MarioRodrigues10

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MarioRodrigues10 @joaodiaslobo @ruilopesm