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/starter sidebar scroll #8743

Merged

Conversation

amberleyromo
Copy link
Contributor

@amberleyromo amberleyromo commented Oct 3, 2018

Related to @LekoArts observation about the sidebar filters on starter library. (Had been bothering me too).

starter_gif

  • Adds option to set a filter set as "fixed"
  • Evenly distributes height to remaining filters
  • Consistent space for "Reset Filters" button

@amberleyromo amberleyromo requested a review from a team as a code owner October 3, 2018 05:20
@amberleyromo
Copy link
Contributor Author

@LekoArts feedback welcome! thanks for noting this earlier.

Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Looking at the gif (didn’t test it locally) it’s how I wanted to solve it — so yeah, LGTM 😊

Copy link
Contributor

@fk fk left a comment

Choose a reason for hiding this comment

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

👍 Yeah, I have to admit I also was annoyed by the jumping :/

I'm not super happy about the initial unused whitespace when no filters are set. I personally wouldn't mind for the "Reset filters" button to show at the bottom of the sidebar, but moving it into the header
— next to "x Gatsby Starters (filtered)" — might be the better alternative.

@amberleyromo
Copy link
Contributor Author

@fk if it showed up at the bottom of the sidebar we'd have to reserve space at the bottom in order for the "jump" not to happen. We'd have the same problem in a different place. I'm personally a fan of this option, because at least it's visually consistent with the header in the main area on the right.

Glad we could semi-resolve something that had been bothering all of us! 😹

@amberleyromo amberleyromo merged commit 70d041b into gatsbyjs:master Oct 3, 2018
@fk
Copy link
Contributor

fk commented Oct 4, 2018

if it showed up at the bottom of the sidebar we'd have to reserve space at the bottom in order for the "jump" not to happen. We'd have the same problem in a different place.

True…d'oh (Homer Simpson voice) ;-) … wondering now why I subconciously implied that the other jump wouldn't be as bad :-D :-/

Glad we could semi-resolve something that had been bothering all of us!

💯

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