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.

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

Masthead Revision #17927

Merged
merged 60 commits into from
Jun 29, 2024
Merged

Masthead Revision #17927

merged 60 commits into from
Jun 29, 2024

Conversation

guerler
Copy link
Contributor

@guerler guerler commented Apr 7, 2024

The goal of this PR is to achieve consistency and reduce redundancy in the Masthead and align its functionality with the Activity Bar. The PR also contains improvements to the Activity Bar and Masthead as recently discussed during the UX team meeting.

  1. @mvdbeek suggested that hidden and less frequently used activities should still be readily accessible. To account for that, the Settings activity has been replaced with a More activity. Users may still use this activity to add or remove items from the activity bar. Additionally, now users may also directly navigate to any activity using this panel.

  2. @davelopez suggested that the username should be visible to the user and that accessing user preferences should remain in the masthead dropdown. This idea has been implemented, the username has been added to the masthead, user preferences and logout are accessible through the user tab.

Resolves #17476, #17109, #17513.

Screen.Recording.2024-06-01.at.8.26.10.AM.mov

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@guerler guerler force-pushed the masthead_update branch 3 times, most recently from c1e2d21 to 88a981b Compare May 21, 2024 15:28
@guerler guerler changed the title Explores revising and removing redundant Masthead items with regard to the Activity bar Revises and removes redundant Masthead items with regard to the Activity bar May 28, 2024
@guerler guerler added this to the 24.2 milestone Jun 1, 2024
@guerler guerler changed the title Revises and removes redundant Masthead items with regard to the Activity bar Masthead Revision Jun 3, 2024
@dannon dannon self-requested a review June 4, 2024 16:27
@guerler guerler marked this pull request as ready for review June 5, 2024 05:01
@martenson
Copy link
Member

martenson commented Jun 28, 2024

The storage usage graph gained more prominence because everything around has disappeared and it also got bigger. It is now a dominant masthead item and I am not sure this information is that important, especially if you are way under the limit. I would support only showing the graph in cases where you reach certain threshold (maybe 75% of quota) and even in those cases I'd advocate for making it smaller. It is already outstanding, it does not have to be large.

In cases such as below, where I am far from hitting the limit, a simple stat such as Using 19b without extra visuals would imho suffice.

Screenshot 2024-06-28 at 12 11 58 PM

minor bug: Clicking on the storage graph as an anon user yields a client error, the link is supposed to be disabled without login.

@guerler
Copy link
Contributor Author

guerler commented Jun 28, 2024

@martenson thanks for catching the anon/disable dashboard issue. I added a commit to fix it. Regarding further adjustments to the quota meter, I do actually like that it is displayed consistently and does not switch between just text and progress bar. I did try different sizes but was not entirely satisfied with the result, however we could have a focussed follow-up trying to improve its appearances in a separate PR moving forward. In this PR the focus was particularly on modernizing the underlying component and displaying it consistently.

@martenson
Copy link
Member

Displaying username in masthead should probably trigger trimming at some point.
Screenshot 2024-06-28 at 1 28 51 PM

@martenson
Copy link
Member

martenson commented Jun 28, 2024

couple more small things:

  • the brand is misaligned
Screenshot 2024-06-28 at 3 13 01 PM
  • clicking the masthead question mark first loads the help page, but then it follows with a hard refresh that is unnecessary
  • clicking the GTN hat results in a runtime error:
Uncaught runtime errors:
×
ERROR
Cannot read properties of null (reading 'getElementsByTagName')
TypeError: Cannot read properties of null (reading 'getElementsByTagName')
    at HTMLIFrameElement.<anonymous> (<anonymous>:128:41)
  • the link to data libraries does not fit the location anymore, I'd advocate moving it to the AB even as a dumb link and not an activity

@guerler
Copy link
Contributor Author

guerler commented Jun 28, 2024

@martenson thanks for the detailed review, added a commit to improve the brand alignment. The question mark click refresh issue was fixed in earlier commits. The GTN issue is related to the webhook implementation and unrelated to this PR.

@guerler guerler merged commit dfa683e into galaxyproject:dev Jun 29, 2024
53 of 55 checks passed
@guerler
Copy link
Contributor Author

guerler commented Jun 29, 2024

Thanks for the review, I will follow-up with some of the suggestions regarding moving the data libraries to the activity section and revising the about page!

@guerler guerler deleted the masthead_update branch June 29, 2024 08:26
@martenson
Copy link
Member

This is great @guerler ! I really like the clean masthead and focus on AB for the functionality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

clarify the visual language of selected masthead vs activity bar
3 participants