Skip to content

Conversation

Polleps
Copy link
Member

@Polleps Polleps commented Sep 16, 2022

PR Checklist:

  • Link to related issue closes Improved drive switcher #209
  • Add changelog entry linking to issue
  • Added tests (if needed)
  • (If new feature) added in description / readme

TODO:

  • WebSocket shows disconnected on full page reload (add a test for this)
  • Maybe add the new button to the dropdown list?
  • If the user is not signed in, they should be told that they should, because then they can save their drives! We need some sort of explanation on what a Drive is. This is also missing in the docs.
  • The button consistency is off. The drive switcher button is round with a border, the SideBarItem buttons are rounded and filled with an overlay.
  • The alignment of the rows is off. Top item has too much padding, text is not centrally aligned.

@Polleps Polleps self-assigned this Sep 16, 2022
@joepio
Copy link
Member

joepio commented Sep 16, 2022

please rebase first on main, your PR contains many of the same changes

@Polleps Polleps force-pushed the #209-better-drive-switcher branch from 1694bf8 to a98a793 Compare September 16, 2022 10:36
@joepio
Copy link
Member

joepio commented Sep 16, 2022

ok, good stuff! Switcher is definitely an improvement. But it's not done:

todo list moved to OP

@Polleps
Copy link
Member Author

Polleps commented Sep 16, 2022

ok, good stuff! Switcher is definitely an improvement. But it's not done:

  • WebSocket shows disconnected on full page reload (add a test for this)
  • Some default drives (such as localhost) look a bit weird if they don't work. Perhaps we should hide these defaults if they are offline, or only show them if the user is in fact visiting from localhost (add a test for this)
  • The order of the list changes, the current one is always at the top. I think the order needs to be stable.
  • Maybe add the new button to the dropdown list?
  • If the user is not signed in, they should be told that they should, because then they can save their drives!
  • We need some sort of explanation on what a Drive is. This is also missing in the docs.
  • The button consistency is off. The drive switcher button is round with a border, the SideBarItem buttons are rounded and filled with an overlay.
  • The alignment of the rows is off. Top item has too much padding, text is not centrally aligned.
  1. Strange, I did not notice this locally, will look into it.

  2. Localhost is only visible in development, localhost:5137 is just the origin so it would not look weird in production. The only case where a drive would look weird (eg, a red url) would be when a user manually adds a broken drive or other resource to their agents drives property.

  3. The order of the list changes because it is a history of drives with a max of 5 drives. Maybe this can be better indicated by using a clock icon instead of the checkbox.

  4. I don't think making a new drive will be that common of a use-case to have a dedicated option in the dropdown.

5 & 6) Good point

  1. Dammit forgot to fix it again 😅

  2. Oops

@joepio joepio force-pushed the #209-better-drive-switcher branch from f1d2b5f to 04c3ea5 Compare September 16, 2022 18:27
@Polleps Polleps force-pushed the #209-better-drive-switcher branch from b8f22a8 to ee93346 Compare September 22, 2022 13:51
@joepio joepio marked this pull request as ready for review September 23, 2022 11:51
@joepio joepio merged commit 0d3c366 into main Sep 23, 2022
@joepio joepio deleted the #209-better-drive-switcher branch September 23, 2022 11:53
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.

Improved drive switcher

2 participants