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

Truncated long value names in confirmation window #8670

Merged
merged 6 commits into from
Feb 18, 2025

Conversation

Annaseli
Copy link
Contributor

Closes #8669

Issue

Long user names overflowed "Confirmation" window

Fix

  1. Applied truncation for long values in selected section modals.
  2. Users can hover to see full value and copy the truncated value.

Changes Made

In "Users" Page

  1. create long user name under "Users"
  2. Go to Administration -> Users -> click on the long user name -> Access Credentials tab -> Create Access Key

The issue screenshot:

long_user_name_in_confirm_issue

The fix screenshot:

long_user_name_in_confirm_fix

Changed in file:

webui/src/pages/auth/users/user/credentials.jsx (getMsg function)

Testing

Verified on lakeFS OSS using local DB & object store & ACL

@Annaseli Annaseli added area/UI Improvements or additions to UI include-changelog PR description should be included in next release changelog labels Feb 16, 2025
@Annaseli Annaseli requested review from itaigilo and guy-har February 16, 2025 16:52
Copy link

E2E Test Results - Quickstart

11 passed

Copy link

E2E Test Results - DynamoDB Local - Local Block Adapter

14 passed

Copy link
Contributor

@itaigilo itaigilo left a comment

Choose a reason for hiding this comment

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

I think it's better to keep the ? on the same line.

>
{email}
</strong>
<br/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the newline after the email?
I think it's better to have it on the same line...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was wondering about that. Okay, I'll move the question mark to the same line as the email.

A related question: In your opinion, what should the expected behavior be?
Currently, long emails wrap to a new line, while short emails remain on the same line. Guy thinks the behavior should be the same for short, long, and overly long (requiring truncation) emails in terms of the line on which they appear.

What do you think should be the standard behavior for all cases?

I believe it would be better if all emails—whether short, long, or too long (requiring truncation)—appear on a new line, with the question mark positioned next to them.

Currently, here's what happens:
image

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be the same - right after the email/name, with no newline.
Any good reasons for the newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. You're right—the question mark should be placed immediately after the email/name, so I fixed that.
  2. The second question is: Do we agree that regardless of whether the email/name is short, long, or too long, the email/name should always be placed on a new line (with the question mark right after the email/name)? This way, all cases will appear as shown in these images. In other words, the length of the email/name won't determine whether it stays on the same line as "Create..." or moves to the next line.

image
image
image

Instead of the previous behavior, where a short name like "admin" remained on the same line, while a long email moved to the next line.

short email/name:
image

long email/name:
image

Copy link
Contributor

@itaigilo itaigilo left a comment

Choose a reason for hiding this comment

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

Nice fix 😄

>
{email}
</strong>
{" "}?
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicking again:
I would've removed the {" "}, unless there's a good reason.
But that's your decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it looks better without it :)

@Annaseli Annaseli merged commit 7b26bab into master Feb 18, 2025
39 checks passed
@Annaseli Annaseli deleted the fix/long-name-in-confirmation-window-8669 branch February 18, 2025 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI Improvements or additions to UI include-changelog PR description should be included in next release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: A long value for a user name causes overflow in the credential window
2 participants