aquariumbio / aquarium

The Aquarium Lab Operating System
http://klavinslab.org/aquaverse/
MIT License
57 stars 15 forks source link

V3 manager #651

Closed marikoja closed 3 years ago

marikoja commented 3 years ago

Resolves #558 Resolves #587 Resolves #570 Styling changes and refactors for reusability

gnomicosuw commented 3 years ago

Looks good!

A. Some notes and comments for discussion only

  1. I implemented "globalClasses" on the GroupsPage. I think you just missed that one by mistake :) No worries. (I pushed it this morning).

  2. I'd like to add a card for the loading page and start using it (haha, the page, not the card). I would leave the UX up to Sean (spinner, progress bar, overlay, no overlay, etc...), but some pages are slow to load on my dev box and it "feels" awkward. (The same pages might be a lot faster on a production box?)

  3. I'd like to implement the sign-in popup, it is still very confusing when pages are blank and the user has no clue their token has expired. I can do that when I merge this into my samples branch.

B. Some things we may want to review with Ben before we merge

  1. RE: tablets & useWindowDimensions() - you are changing the style to make the font-size smaller on tablets. Unless that is what Sean wanted, I wouldn't do anything different on "smaller" screens right now!

    • If we do anything then it should probably be a global setting and not just for this page
    • And even on this page it is weird. For example, the side nav bar actually gets WIDER on SMALLER devices, and the font size in the side nav bar does not change...
  2. There are issues with the scroll bars

    • there is white space below the horizontal scroll bar on the bottom
    • there is white space to the right of the vertical scroll bar on the left
  3. Jobs Page:

    • the ListItem links (e.g., the "jobs" and "operations" in the side nav bar) do not have cursor set as pointer. This should really be done.

All that said, if Ben is OK merging as is then I'm OK merging so we can move forward...

gnomicosuw commented 3 years ago

A couple of comments

  1. I had missed a few places where I needed to use globalClasses for flex. I just pushed those
  2. The vertical scrollbar is gone on the hamburger menu pages. I probably need to use the "new" template classes you have defined. I'll take a look before the standup.