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

Style updates to bookmark manager per @bradleyrichter #4690

Merged
merged 3 commits into from
Oct 13, 2016
Merged

Style updates to bookmark manager per @bradleyrichter #4690

merged 3 commits into from
Oct 13, 2016

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented Oct 11, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Fixes #4562
Fixes #2652

Auditors: @jkup @bbondy

Test Plan:

  1. Launch bookmarks manager
  2. Ensure it looks like the latest mockup here: Update styles for Bookmark Manager #4562
  3. Try resizing the window
  4. Enjoy 😄

@bsclifton
Copy link
Member Author

bsclifton commented Oct 12, 2016

Here's a screenshot showing the flexability of the folders area, which fixes #1187

It has a min-width of 220px and a flex-grow of 1 (versus the other having 5).
Adding more levels of folders will extend the size

How low can you go?
screen shot 2016-10-12 at 12 05 33 am

@bbondy
Copy link
Member

bbondy commented Oct 12, 2016

@bradleyrichter could you review?

@srirambv
Copy link
Collaborator

srirambv commented Oct 12, 2016

@bsclifton The import button doesn't seem to be placed correctly compared to the one above, in the preview build
image

@srirambv
Copy link
Collaborator

Long folder names getting clipped

image

@bsclifton
Copy link
Member Author

@srirambv those issues will be fixed with this PR 😄 This should also fix a failing test

@bradleyrichter
Copy link
Contributor

Ideally we would have ellipses after a max width and a full-text on hover.

Later, drag-adjust column widths.

Please don't force the column width to fit the long folder names.

@bsclifton
Copy link
Member Author

@bradleyrichter should there be a max width setting? Adding that would be fairly easy. Overflow can be handled with either ellipsis or by adding a horizontal scroll bar at the bottom

@bsclifton
Copy link
Member Author

bsclifton commented Oct 12, 2016

Screen caps of some of the changes we did:

New "All Folders"

shows when you do a search (otherwise hidden):
screen shot 2016-10-12 at 3 04 26 pm

Demo of the new color highlights for sortableTable

cc: @darkdh
light gray = hover over item
medium gray = selected item
dark orange = hover over selected item
screen shot 2016-10-12 at 3 19 15 pm

See #4562 to see some of the proposed UI changes

Auditors: @jkup or @bbondy

Test Plan:
1. Launch bookmarks manager
2. Ensure it looks like the latest mockup here: #4562
Bookmarks Manager
- searching now shows "All Folders" with magnifying glass at top of folder list
- if search is done, selected folder doesn't show as picked
- colors tweaked for hover/selected in folder list
- reduced size of bookmark import icon, updated color to @buttonColor
- "Folder" text no longer selectable

Sortable table (affects about:history and preferences > search)
- Highlight color changed (was orange, now is gray, matching folder selection)
- Hover color changed (was light yellow/orange, now is light gray)
- Highlight/hover text color changed to be @buttonColor
- Box shadow and border removed

History page
- Entries are no longer selectable. Often, multi-select would select the actual text too

Auditors: @bradleyrichter
@jkup
Copy link
Contributor

jkup commented Oct 12, 2016

Looks great. A few tiny things:

  1. The page has a scrollbar even with only 1 item.
  2. Should we center the search bar on smaller devices?
  3. Is it ok that the titles aren't scrollable on smaller devices? Not sure if it matters but it means you can't read the entire thing.

Pictures for 2 & 3
screen shot 2016-10-12 at 4 26 53 pm
screen shot 2016-10-12 at 4 26 57 pm

@bsclifton
Copy link
Member Author

bsclifton commented Oct 13, 2016

Sweet- thanks for the feedback, @jkup! I'll can work on a fix for 1 and 3 in a follow up 😄

@bradleyrichter what do you think about 2? (should the search box be centered if you reduce the window size- see @jkup's pic)

@bsclifton
Copy link
Member Author

I'll go ahead and merge- @bradleyrichter I can address those items in a follow up (0.12.6, when adding folders). Please let me know!

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.

7 participants