Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Split scroll between titles and folders for BM manager #8532

Merged
merged 1 commit into from
May 1, 2017

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented Apr 27, 2017

Test Plan:

  • Have a lot of bookmarks
  • Go to about:bookmarks
  • Ensure that:
    • Layout is fixed
    • You can scroll between folders (left-hand side column) and titles (right-hand side column) wouldn't scroll together
    • Less-than-screen-height number of bookmakrs shouldn't let you scroll (layout is fixed)
    • At small sizes we fallback to what we have and you can scroll gracefully

Description

@cezaraugusto
Copy link
Contributor Author

/cc @bradleyrichter for design review. Scrollbar is the same used for #8475

@cezaraugusto cezaraugusto self-assigned this Apr 27, 2017
Copy link
Collaborator

@jonathansampson jonathansampson left a comment

Choose a reason for hiding this comment

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

Can we prevent the Title and Last Visited headers from scrolling with the content? Also, thoughts on adding an overflow-ellipsis to the title entries?

bookmarks-headers

@cezaraugusto
Copy link
Contributor Author

cezaraugusto commented Apr 28, 2017

@jonathansampson yep that's the ideal but I can't since it uses <table> (SortableTable component) which doesn't accept overflow.

Ideally sortableTable should make use of css-grid to keep it's table-like look, but changes would be too bulk and imo deserves a standalone refactor.

++ for ellipsis. I'll do another round on this since Brad don't want custom sidebars and include that, thanks!

@jonathansampson
Copy link
Collaborator

@cezaraugusto All valid points. I gladly approve, provided we can create an issue to refactor the bookmarks items portion for frozen headers.

@bradleyrichter
Copy link
Contributor

This is great to have, even with the unresolved header.

@cezaraugusto
Copy link
Contributor Author

ok based on #8532 (comment) I created an issue to track fixed header for bm items: #8607. Also added ellipsis for long texts.

I also removed completely ScrollableContent element since component was created to address #7930 and this issue, and is no longer required.

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Huge ++ on this 😄 Looks and feels awesome

This functionality was originally there but it was lost when I refactored the page into its current state

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Folders and Titles lists in about:bookmarks should scroll independently
5 participants