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

Refactor Sidebar #4019

Merged
merged 1 commit into from Jul 12, 2023
Merged

Refactor Sidebar #4019

merged 1 commit into from Jul 12, 2023

Conversation

falbru
Copy link
Contributor

@falbru falbru commented Jul 4, 2023

Description

  • Refactor the Sidebar component to utilize a hook
  • The previous JSX and CSS was a mess. This looks much cleaner imo <3
  • Make the sidebar fixed in mobile view in order to remove dead space.
  • Fix a bug where the sidebar in mobile view would overflow vertically if too many tabs where opened

This sidebar looks basically the same for desktop view.

Result

Desktop view

Proof that they look about the same:

Before
before

After
after

Mobile view

By making the sidebar fixed, you remove the "dead space" that would appear if a page was too long. This makes it so that the user doesn't have to scroll to the top of the page when opening the sidebar. It preserves the scroll of the page between opening and closing the sidebar.

Before

Screencast_20230704_222749-2.webm

After

Screencast_20230704_222749-3.webm

It also fixes this overflow bug as described earlier:
Screenshot_20230704_221345

Testing

  • I have thoroughly tested my changes.

I have tested it with many different window sizes and tabs in the sidebar collapsed.


@falbru falbru added the enhancement Pull requests that make enhancements, instead of just purely new features label Jul 4, 2023
@github-actions github-actions bot added the review-needed Pull requests that need review label Jul 4, 2023
@falbru falbru force-pushed the refactor-sidebar branch 2 times, most recently from 591a282 to 91b5ae5 Compare July 4, 2023 20:44
Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

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

A big improvement! 😍 I love PRs like these!!

app/routes/pages/components/Sidebar.css Outdated Show resolved Hide resolved
app/routes/pages/components/Sidebar.tsx Outdated Show resolved Hide resolved
app/routes/pages/components/Sidebar.tsx Show resolved Hide resolved
@ivarnakken ivarnakken added approved Pull requests that have been approved technical-debt Pull requests that reduces technical debt labels Jul 4, 2023
- Refactor the Sidebar component to be a hook
- The previous JSX and CSS was a mess. This looks much cleaner imo <3
- Make the sidebar fixed in mobile view in order to remove dead space.
- Fix a bug where the sidebar in mobile view would overflow vertically if too many tabs where opened
@falbru falbru merged commit 7e46488 into master Jul 12, 2023
4 checks passed
@falbru falbru deleted the refactor-sidebar branch July 12, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved enhancement Pull requests that make enhancements, instead of just purely new features review-needed Pull requests that need review technical-debt Pull requests that reduces technical debt
Projects
None yet
2 participants