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-388: Handle text overflow on longer system labels(systemName) #901

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

tjgrafft
Copy link
Collaborator

Overview

The system name (root) in the dropdown menu is not truncated like the path folder names. This is causing a text overflow if the system name is a long string. See example below with Frontera system names/labels. Only an issue if the system name is over 20-char in length.
Screenshot 2023-11-14 at 9 46 41 AM

We can use the substring method, like I used for the path folder names below to handle this issue.
Screenshot 2023-11-14 at 9 52 35 AM

Related

Changes

  • Added check if systemName was longer than 20 characters--used substring method to grab first 20 chars--otherwise render full systemName (or Shared Workspaces, if applicable)

Testing

  1. Go to server > portal > settings > settings_default.py
  2. Locate _PORTAL_DATAFILES_STORAGE_SYSTEMS
  3. Change the name field for one of the objects to something like 'name': 'My Data (Frontera Scratch)'
  4. Go to cep.test
  5. Look in your Data Files to find the new system label you changed in settings
  6. Open the Dropdown menu and check to make sure the long system label is no longer overflowing.
  7. Compare with picture below
  8. Change the name field back to the original label :)

UI

Screenshot 2023-11-15 at 2 04 05 PM

Notes

@tjgrafft tjgrafft changed the title fixed text overflow for longer system labels(systemName) task/WP-388: fixed text overflow for longer system labels(systemName) Nov 15, 2023
Copy link

codecov bot commented Nov 15, 2023

Codecov Report

Merging #901 (8654a0e) into main (d343e74) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #901   +/-   ##
=======================================
  Coverage   63.40%   63.40%           
=======================================
  Files         432      432           
  Lines       12403    12403           
  Branches     2584     2584           
=======================================
  Hits         7864     7864           
  Misses       4327     4327           
  Partials      212      212           
Flag Coverage Δ
javascript 69.75% <ø> (ø)
unittests 56.93% <ø> (ø)

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

@rstijerina
Copy link
Member

Should we expand the width of the dropdown window to fit longer system names @wesleyboar ?

@wesleyboar
Copy link
Member

@rstijerina, making menu wider is fine.

@tjgrafft General note: If you use CSS for truncation (instead of JavaScript), then you never need to worry about character count. Browser will do the math for you.

@tjgrafft
Copy link
Collaborator Author

@wesleyboar Good point. Personally I didn't want to use truncate middle or the other truncate util I used for folder names bc I didn't think the ellipsis would look good in the dropdown menu, but I could make one without the ellipsis ending. Or I can have the dropdown menu autofit the width based on systemName. I'll look into both. Thanks!

@tjgrafft
Copy link
Collaborator Author

Screenshot 2023-11-15 at 4 00 09 PM
New look with auto width css

@tjgrafft tjgrafft requested a review from wesleyboar November 15, 2023 22:04
@@ -13,7 +13,8 @@ Styleguide Components.Dropdown
border-radius: 0;
margin-top: 11px;
padding: 0;
width: 200px;
min-width: 200px;
width: auto;
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a max-width, for very long folder names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The path folder names won't be an issue bc they're being restricted to 20 chars. So it would only be an issue with system names/labels. My thinking is if we include a max-width the text overflow would happen again with a long system name. I know there's already code in the css file to handle media/device screen size rules. Do we think that's enough to handle the potential issue, or should I add a max-width of 800px and assume we'd never have a super long system name label past that?

Copy link
Member

Choose a reason for hiding this comment

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

Ah perfect, I think this is a good solution to this problem. Very long system names are unlikely, but we could address that in another task if need be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good!

@rstijerina rstijerina merged commit d54517b into main Nov 20, 2023
6 checks passed
@rstijerina rstijerina deleted the task/WP-388-systemName-overflow-fix branch November 20, 2023 19:34
@chandra-tacc chandra-tacc changed the title task/WP-388: fixed text overflow for longer system labels(systemName) task/WP-388: Handle text overflow on longer system labels(systemName) Nov 27, 2023
chandra-tacc pushed a commit that referenced this pull request Dec 8, 2023
…#901)

* fixed text overflow for longer system labels(systemName)

* added auto-width css

---------

Co-authored-by: Taylor Grafft <[email protected]>
Co-authored-by: Sal Tijerina <[email protected]>
chandra-tacc pushed a commit that referenced this pull request Dec 8, 2023
…#901)

* fixed text overflow for longer system labels(systemName)

* added auto-width css

---------

Co-authored-by: Taylor Grafft <[email protected]>
Co-authored-by: Sal Tijerina <[email protected]>
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.

3 participants