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

Long email addresses cause incorrect formatting of the left sidebar change history - resolved #254

Conversation

aiAdrian
Copy link
Collaborator

@aiAdrian aiAdrian commented Aug 19, 2024

Description

before

image

after fix

image

The user can scroll by selecting to copy the email address.

Checklist

  • This PR contains a description of the changes I'm making
  • I've read the Contribution Guidelines
  • I've added tests for changes or features I've introduced
  • I documented any high-level concepts I'm introducing in documentation/
  • CI is currently green and this is ready for review

@aiAdrian aiAdrian self-assigned this Aug 19, 2024
Copy link
Collaborator

@louisgreiner louisgreiner left a comment

Choose a reason for hiding this comment

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

Otherwise, LTGM and tested

.user-id {
width: 140px; /* Set the desired width */
overflow: auto; /* Hide overflowing content */
text-overflow: ellipsis; /* Display ellipsis (...) for overflow */
Copy link
Collaborator

Choose a reason for hiding this comment

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

As the input field doesn't show "asupersuperlongem..."but "asupersuperlongem", you might just comment that the email overflows, but doesn't display "..."

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have overworked the "UX concept" - now the email address gets copied and the log service shows a toast to inform you that the email address was copied. The copied function is as well active, when user clicks on the comments.

@aiAdrian aiAdrian requested review from louisgreiner and removed request for emersion August 20, 2024 07:59
…netzgrafik-editor-frontend into 243-bug-long-email-addresses-cause-incorrect-formatting-of-the-left-sidebar-change-history
…ows a toast to inform the user that the email address was copied.
…netzgrafik-editor-frontend into 243-bug-long-email-addresses-cause-incorrect-formatting-of-the-left-sidebar-change-history
Copy link
Collaborator

@louisgreiner louisgreiner left a comment

Choose a reason for hiding this comment

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

Tested and working very well

Comment on lines +57 to +59
navigator.clipboard.writeText(userId);
const msg = $localize`:@@app.view.variant.variant-view.variant-history.version-entry-layout.user-id-copied:copied ${userId}:msg:`;
this.logService.info(msg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change: use await navigator.clipboard.writeText() to only show the info message once the text has successfully been copied to the clipboard. (In some cases it can take a while or fail.)

Copy link
Collaborator

@emersion emersion left a comment

Choose a reason for hiding this comment

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

Apart from this looks good!

@aiAdrian aiAdrian merged commit dd0fda5 into main Aug 20, 2024
6 checks passed
@aiAdrian aiAdrian deleted the 243-bug-long-email-addresses-cause-incorrect-formatting-of-the-left-sidebar-change-history branch August 20, 2024 11:18
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.

[Bug]: Long email addresses cause incorrect formatting of the left sidebar change history
3 participants