Skip to content

Redesign profile switcher #51419

Merged
gzdunek merged 27 commits intomasterfrom
gzdunek/redesign-profiles
Jan 30, 2025
Merged

Redesign profile switcher #51419
gzdunek merged 27 commits intomasterfrom
gzdunek/redesign-profiles

Conversation

@gzdunek
Copy link
Copy Markdown
Contributor

@gzdunek gzdunek commented Jan 23, 2025

This is the part 2/2 of the redesigned profile switcher. The aim of this redesign is to address two issues:

  • Truncating the cluster name. Currently, items on the list have the format of user@cluster, so when they are long, they become truncated. We can solve this by putting them in separate lines.
  • It’s challenging to quickly differentiate clusters. Currently, we display the first letter of a username in a purple circle in the top-right corner. However, if you use the same username across all clusters, this indicator remains identical, making it difficult to distinguish between them. To improve it, we could allow the users to assign custom colors to the “avatars”.

This PR is quite large (unfortunately, I also had to refactor the entire Identity story). I tried to extract smaller commits, but it was difficult because there were a lot of changes in all the components. To reduce the number of changed lines, I tried to keep the components in their original files, but I'm going to move things around and rename them in the cleaning PR.

The thing that could be better is accessibility. These list items should probably be buttons to improve keyboard navigation, but since we have logout buttons inside them, I got a HTML error about nesting button inside another button, so I stayed with li and made it tabbable.
Also selecting a color should work better with a keyboard. I will try to improve this one.

Before going through the code, I would like to ask you to test the app first. I would like to get your feedback on the redesigned switcher, perhaps you will find that something does not look / work well, so I will update the implementation.

Changelog: Redesigned the profile switcher in Teleport Connect for a more intuitive experience. Clusters now have distinct colors for easier identification, and readability is improved by preventing truncation of long user and cluster names.

Demo:

new.profile.switcher.mov

Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

I'll continue the review tomorrow.

Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/ColorPicker.tsx Outdated
Comment thread web/packages/teleterm/src/ui/services/workspacesService/profileColor.ts Outdated
Comment thread web/packages/design/src/theme/themes/darkTheme.ts
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx Outdated
@ravicious ravicious self-requested a review January 27, 2025 14:17
Copy link
Copy Markdown
Contributor

@avatus avatus left a comment

Choose a reason for hiding this comment

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

This works really great for me locally. I love it. I think a min width a little larger would be good for my taste but i know thats subjective.
Screenshot 2025-01-27 at 11 17 39 AM

i know this isnt really crowded but it feels slightly crowded to me.

at any rate, it works great and i think its a nice addition

Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

I got to "Put the new identity components together", will continue tomorrow.

Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx Outdated
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentitySelector/UserIcon.tsx Outdated
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityListItem.tsx Outdated
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx Outdated
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityList.tsx Outdated
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx Outdated
@ravicious ravicious self-requested a review January 28, 2025 13:50
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/Identity.story.tsx
Comment thread web/packages/teleterm/src/ui/TopBar/Identity/Identity.story.tsx
@gzdunek gzdunek requested a review from ravicious January 29, 2025 17:37
Copy link
Copy Markdown
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

Good job!

Comment thread web/packages/teleterm/src/ui/TopBar/Identity/IdentityList/IdentityListItem.tsx Outdated
@gzdunek gzdunek force-pushed the gzdunek/redesign-profiles branch from 3c172ba to 74720ad Compare January 30, 2025 14:06
Base automatically changed from gzdunek/store-profile-color to master January 30, 2025 15:39
@gzdunek gzdunek force-pushed the gzdunek/redesign-profiles branch from 74720ad to 2e6b4a4 Compare January 30, 2025 15:42
@gzdunek gzdunek enabled auto-merge January 30, 2025 15:43
@gzdunek gzdunek added this pull request to the merge queue Jan 30, 2025
Merged via the queue into master with commit 937e4a9 Jan 30, 2025
@gzdunek gzdunek deleted the gzdunek/redesign-profiles branch January 30, 2025 16:00
@public-teleport-github-review-bot
Copy link
Copy Markdown

@gzdunek See the table below for backport results.

Branch Result
branch/v17 Failed

gzdunek added a commit that referenced this pull request Jan 30, 2025
* Add mapping of profile color to theme color

* Prepare UserIcon to display colors, make it slightly larger

* Rewrite single identity item to show icon and user/cluster in separate lines

* Render single cluster items in a list

* Add color picker

* Render active cluster separately, above inactive clusters

* Put the new identity components together

* Refactor `Identity` story, render the container component instead of "presentational" one

* Refresh startup page

* Minor style fixes

* Add comment about `dataVisualisationColors`

* Bring back the original styling of `StaticListItem`

* Make the pencil icon hoverable and clickable

* Increase min-width from 200 px to 300 px, simplify active cluster layout

* Show logout button when any part of `ListItem` is hovered

* Remove text from logout button, improve titles

* Pass correct index number to `AddClusterItem`

* Update comment for `focusGrabber`

* Use `useStoreSelector` correctly

* Improve accessibility of color picker

* Replace Pam icon with plus symbol

* Remove unnecessary optional chaining

* Align profile status error with user icon

* Add story for cluster connect panel

* Make user icon letter follow active theme

This was suggested by Kenny, and I think it looks good.

* Change gap from 11 px to 10 px

* Rename profile color -> workspace color

(cherry picked from commit 937e4a9)
github-merge-queue Bot pushed a commit that referenced this pull request Jan 30, 2025
* Store profile color in workspace (#51417)

* Adjust workspace tests to accept persisted workspace type

* Store profile color in workspace

* Replace manual workspace creation in stories/tests with `addRootClusterWithDoc`

* Fix checking if all colors are taken

* Add DELETE IN comment

* `ProfileColor` -> `WorkspaceColor`

* Lint

* Prettier

* Add TODO about creating workspaces in `setActiveWorkspace`

(cherry picked from commit 3b1c689)

* Redesign profile switcher  (#51419)

* Add mapping of profile color to theme color

* Prepare UserIcon to display colors, make it slightly larger

* Rewrite single identity item to show icon and user/cluster in separate lines

* Render single cluster items in a list

* Add color picker

* Render active cluster separately, above inactive clusters

* Put the new identity components together

* Refactor `Identity` story, render the container component instead of "presentational" one

* Refresh startup page

* Minor style fixes

* Add comment about `dataVisualisationColors`

* Bring back the original styling of `StaticListItem`

* Make the pencil icon hoverable and clickable

* Increase min-width from 200 px to 300 px, simplify active cluster layout

* Show logout button when any part of `ListItem` is hovered

* Remove text from logout button, improve titles

* Pass correct index number to `AddClusterItem`

* Update comment for `focusGrabber`

* Use `useStoreSelector` correctly

* Improve accessibility of color picker

* Replace Pam icon with plus symbol

* Remove unnecessary optional chaining

* Align profile status error with user icon

* Add story for cluster connect panel

* Make user icon letter follow active theme

This was suggested by Kenny, and I think it looks good.

* Change gap from 11 px to 10 px

* Rename profile color -> workspace color

(cherry picked from commit 937e4a9)
carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
* Add mapping of profile color to theme color

* Prepare UserIcon to display colors, make it slightly larger

* Rewrite single identity item to show icon and user/cluster in separate lines

* Render single cluster items in a list

* Add color picker

* Render active cluster separately, above inactive clusters

* Put the new identity components together

* Refactor `Identity` story, render the container component instead of "presentational" one

* Refresh startup page

* Minor style fixes

* Add comment about `dataVisualisationColors`

* Bring back the original styling of `StaticListItem`

* Make the pencil icon hoverable and clickable

* Increase min-width from 200 px to 300 px, simplify active cluster layout

* Show logout button when any part of `ListItem` is hovered

* Remove text from logout button, improve titles

* Pass correct index number to `AddClusterItem`

* Update comment for `focusGrabber`

* Use `useStoreSelector` correctly

* Improve accessibility of color picker

* Replace Pam icon with plus symbol

* Remove unnecessary optional chaining

* Align profile status error with user icon

* Add story for cluster connect panel

* Make user icon letter follow active theme

This was suggested by Kenny, and I think it looks good.

* Change gap from 11 px to 10 px

* Rename profile color -> workspace color
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants