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

task/WP-273: Category icon #874

Merged
merged 13 commits into from
Oct 25, 2023
Merged

task/WP-273: Category icon #874

merged 13 commits into from
Oct 25, 2023

Conversation

tjgrafft
Copy link
Collaborator

@tjgrafft tjgrafft commented Oct 4, 2023

Overview

Tracy discussed with Wes for design.

Any portal that has Visualization as a category should use the cube icon. For apps in that category, if they do NOT have a unique icon for the app, then use the CATEGORY icon as the default. So default apps would have a cube here too.

Check the following portals in staging/dev/pprd and prod -
3DEM, A2CPS, CEP, Frontera, UTRC, and PTdatax (once converted)

Related

Changes

  • For applications in a category (Visualization, Data Processing, Utilities, etc), if they don't have a custom/unique icon for that application, they will now use the category icon for that application's icon.
  • Added a new custom icon for the icon-utilities category (after discussion with Tracy) so that if someone were to add an application in the Utilities category, that doesn't have its own unique icon--it will default to the icon-utilities category icon (which happens to be the same icon as icon-applications, the four squares).
  • Added a new category prop to the AppIcon component. This allows the applications to get the correct category (including the icon they're in)
  • Added a function in AppInfo component, in the AppForm file. This function, getAppCategory, gets the correct category for each application. This was necessary so that I could pass that data into the category prop for AppIcon and get the correct icon for the AppForm (documentation link associated with the selected/active app)
  • Added new unit tests for making sure the category for an application is correct--especially to test the case when there's no unique icon for an application (wanted to make sure it used the category icon instead).

Testing

  1. Go to https://cep.test/workbench/applications
  2. To really see the benefits of the new code, go to https://cep.test/core/admin/ (assuming you have admin powers setup locally...if not you can set it up by following these instructions https://github.com/TACC/Core-Portal/wiki/How-to-Set-Your-User-as-Staff-or-Superuser)
  3. Scroll down to "App Tray Categories", and click "Add".
  4. Add the "Visualization" category to the tray. Save
  5. Go to "App Tray Entries" (back on https://cep.test/core/admin/) and click "Add".
  6. Make the category "Visualization"
  7. Make the appId "hello-world"
  8. Make the version "0.0.1" Save
  9. Navigate back to https://cep.test/workbench/applications
  10. You should now see the Visualization category. Inside it is the Hello-World application.
  11. It will have the cube icon for the hello-world app. The hello-world app will have an icon className of "icon-visualization".

UI

Visualization_Icon Hello World icon for Visualization Category Simulation_Icon Hello World icon for Simulation Category

Notes

Notice in the second screenshot that the matlab application icon is still its unique icon, however the hello-world icon is the simulation icon bc that application has no custom icon.

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #874 (ec2c370) into main (f3e7f18) will increase coverage by 0.01%.
The diff coverage is 73.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #874      +/-   ##
==========================================
+ Coverage   63.36%   63.38%   +0.01%     
==========================================
  Files         429      430       +1     
  Lines       12271    12292      +21     
  Branches     2532     2541       +9     
==========================================
+ Hits         7776     7791      +15     
- Misses       4288     4292       +4     
- Partials      207      209       +2     
Flag Coverage Δ
javascript 69.74% <73.91%> (+<0.01%) ⬆️
unittests 56.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../components/Applications/AppBrowser/AppBrowser.jsx 86.20% <ø> (ø)
...nt/src/components/Applications/AppForm/AppForm.jsx 83.88% <85.71%> (+0.05%) ⬆️
client/src/components/_common/AppIcon/AppIcon.jsx 95.23% <85.71%> (-4.77%) ⬇️
client/src/utils/doesClassExist.js 55.55% <55.55%> (ø)

client/src/styles/trumps/icon.fonts.css Outdated Show resolved Hide resolved
client/src/styles/trumps/icon.fonts.css Outdated Show resolved Hide resolved
client/src/components/_common/AppIcon/AppIcon.jsx Outdated Show resolved Hide resolved
@rstijerina rstijerina changed the title Task/wp 273 category icon task/WP-273: Category icon Oct 10, 2023
Copy link
Member

@rstijerina rstijerina left a comment

Choose a reason for hiding this comment

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

Looking great! Just two further questions

client/src/styles/trumps/icon.fonts.css Outdated Show resolved Hide resolved
client/src/components/_common/AppIcon/AppIcon.jsx Outdated Show resolved Hide resolved
Copy link
Member

@rstijerina rstijerina left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

LGTM! good PR.
Only one minor comment, sorry for late review on this.

client/src/components/Applications/AppForm/AppForm.jsx Outdated Show resolved Hide resolved
@tjgrafft tjgrafft requested a review from chandra-tacc October 25, 2023 19:47
Copy link
Collaborator

@chandra-tacc chandra-tacc left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for addressing comments.

@chandra-tacc chandra-tacc merged commit 1495914 into main Oct 25, 2023
6 checks passed
@chandra-tacc chandra-tacc deleted the task/WP-273-CategoryIcon branch October 25, 2023 20:14
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.

4 participants