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

Very long tag name break table #217

Open
SimonLab opened this issue Nov 29, 2022 · 8 comments
Open

Very long tag name break table #217

SimonLab opened this issue Nov 29, 2022 · 8 comments
Assignees
Labels
bug Suspected or confirmed bug (defect) in the code chore a tedious but necessary task often paying technical debt please-test Please test the feature in Staging Environment and confirm it's working as expected. priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T25m Time Estimate 25 Minutes tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@SimonLab
Copy link
Member

image

These are tags created on fly.io when not loggedin
Add a max-width and overflow settings on the tag name (using ellipsis could maybe work?)

@SimonLab SimonLab added the bug Suspected or confirmed bug (defect) in the code label Nov 29, 2022
@SimonLab
Copy link
Member Author

The issue is that the tag text is one word without spaces so it overflow the table width
see https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Text/Wrapping_Text

We can see that if we create a long tag name with multiple spaces the tag doesn't overflow:

image

@nelsonic nelsonic added chore a tedious but necessary task often paying technical debt T25m Time Estimate 25 Minutes technical A technical issue that requires understanding of the code, infrastructure or dependencies priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written labels Nov 29, 2022
@SimonLab
Copy link
Member Author

resolved by adding the following classes to the span containing the tag name:
overflow-hidden text-ellipsis whitespace-nowrap inline-block w-40

see also https://stackoverflow.com/questions/17779293/css-text-overflow-ellipsis-not-working
image

SimonLab added a commit that referenced this issue Nov 29, 2022
Add css classes to add ellipsis when tag name is too long to be
displayed in the table

see: #217
@SimonLab SimonLab added the awaiting-review An issue or pull request that needs to be reviewed label Nov 29, 2022
@SimonLab SimonLab moved this to ⏳Awaiting Review in dwyl app kanban Nov 29, 2022
@SimonLab
Copy link
Member Author

Not there yet:
image

The width is applied to all the tags, it work for long text however for smaller text we want the pill width to match the length of the content, tried to apply a max width instead but this doesn't work. I need to investigate a bit more how to fix this:

@SimonLab
Copy link
Member Author

SimonLab commented Nov 29, 2022

The max width are working, I wasn't using the correct tailwind class.
However the default max-width are too long:
https://tailwindcss.com/docs/max-width

Luckily Tailwind provides also a way to have a customised class:
image

Going for max-w-[144px]

image

SimonLab added a commit that referenced this issue Nov 29, 2022
@nelsonic
Copy link
Member

@SimonLab glad you persisting with this UI quest. 🤞🏼
Please consider using a mobile simulator e.g. Firefox to try and appear mobile-first. 🙏📱

@SimonLab
Copy link
Member Author

SimonLab commented Nov 29, 2022

tested on mobile, see #219 (comment)

deployed and fly version fixed:
image

Repository owner moved this from ⏳Awaiting Review to ✅ Done in dwyl app kanban Nov 29, 2022
@SimonLab SimonLab removed the awaiting-review An issue or pull request that needs to be reviewed label Nov 29, 2022
@nelsonic
Copy link
Member

@SimonLab yes, "Tested on mobile" but screenshots still on desktop ... 🖥️
What I mean is: take the screenshot in Firefox with the viewport constrained to a Mobile View e.g: dwyl/dev-setup#55

For a new person there are no default tags so when they attempt to add tags from the "home" view:

mvp-no-tags-by-default

They need to manually go to the /tags page in order to Create Tag:

mvp-create-tag

This is undesirable UX. We definitely don't want to force people to change contexts to create a tag.
This is a "Why?" moment and should be avoided.

@nelsonic nelsonic added the please-test Please test the feature in Staging Environment and confirm it's working as expected. label Nov 29, 2022
@SimonLab SimonLab reopened this Nov 30, 2022
@SimonLab
Copy link
Member Author

SimonLab commented Nov 30, 2022

"Tested on mobile" but screenshots still on desktop ... desktop_computer:

image

We can see the pills length is based on the tag name, except when the name is too long, then ellipsis are added.

This is undesirable UX. We definitely don't want to force people to change contexts to create a tag.

I'll open another issue for this:
#221

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Suspected or confirmed bug (defect) in the code chore a tedious but necessary task often paying technical debt please-test Please test the feature in Staging Environment and confirm it's working as expected. priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T25m Time Estimate 25 Minutes tech-debt A feature/requirement implemented in a sub-optimal way & must be re-written technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: Done
Development

No branches or pull requests

3 participants