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

Collapse collection items in Sidebar #503

Merged
merged 11 commits into from
Oct 4, 2019
Merged

Collapse collection items in Sidebar #503

merged 11 commits into from
Oct 4, 2019

Conversation

ashmaroli
Copy link
Member

To improve UX in sites with numerous collections.

  • Renders as the first item in the Sidebar giving it ample space to expand downwards.
  • Renders a counter showing count of items within panel when collapsed.
  • Renders link to 'Posts' separately.

ashmaroli and others added 4 commits September 30, 2019 11:20
@ashmaroli
Collapse collection items in Sidebar
be981f1 
To improve UX in sites with numerous collections.
  - Renders as the first item in the Sidebar giving it ample space to
    expand downwards.
  - Renders a counter showing count of items within panel when collapsed
  - Renders link to 'Posts' separately
@ashmaroli
Merge branch 'master' into collections-accordion
158bbff
@ashmaroli
Merge branch 'master' into collections-accordion
492b0cc
@ashmaroli
Add translations for UI text "Collections"
1456cc7
mertkahyaoglu
mertkahyaoglu requested changes Oct 2, 2019
View reviewed changes
src/containers/Sidebar.js Outdated Show resolved Hide resolved
src/containers/Sidebar.js Outdated Show resolved Hide resolved
src/containers/Sidebar.js Outdated Show resolved Hide resolved
src/containers/Sidebar.js Outdated Show resolved Hide resolved
src/containers/Sidebar.js Outdated Show resolved Hide resolved
mertkahyaoglu
mertkahyaoglu requested changes Oct 2, 2019
View reviewed changes
src/containers/Sidebar.js Outdated Show resolved Hide resolved
@ashmaroli
Copy link
Member Author

@mertkahyaoglu The diff changed slightly since your last review. Notably,

  • there is no <div /> enclosing the accordion list-item
  • there won't be an empty <div /> rendered when there are no collections + "posts" are hidden + drafts are not rendered.
  • As a side-effect, there will now exist two <Splitter /> if "pages" and "posts" are hidden in a site without drafts and other collections. I plan to resolve that in a separate PR.

Would be great if you could do a re-review.
Thanks.

lexoyo
lexoyo approved these changes Oct 3, 2019
View reviewed changes
Copy link
Collaborator

@lexoyo lexoyo left a comment

Choose a reason for hiding this comment

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

Nice addition! 👍

mertkahyaoglu
mertkahyaoglu approved these changes Oct 4, 2019
View reviewed changes
Copy link
Member

@mertkahyaoglu mertkahyaoglu left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ashmaroli
Copy link
Member Author

Thank you very much, guys 😃

@ashmaroli ashmaroli merged commit 8723f07 into jekyll:master Oct 4, 2019
@ashmaroli ashmaroli deleted the collections-accordion branch October 4, 2019 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lexoyo lexoyo lexoyo approved these changes

@mertkahyaoglu mertkahyaoglu mertkahyaoglu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ashmaroli @lexoyo @mertkahyaoglu