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

Improve dock tabs UX #4754

Merged
merged 59 commits into from
Mar 21, 2022
Merged

Improve dock tabs UX #4754

merged 59 commits into from
Mar 21, 2022

Conversation

DmitriyNoa
Copy link
Contributor

@DmitriyNoa DmitriyNoa commented Jan 25, 2022

Closes: #4459

The current solution is inspired by VSCode and Webstrom tabs management solutions.

When the number of tabs is getting too high the area will be scrollable.

When the number of tabs is getting too high the scroll control are presented (scroll left, and scroll right).

Scroll controls are displayed based on the scroll position.

Tabs are now clearly separated from each other.

The active tab is highlighted and visible.

The double click handler was removed as it sometimes caused accidental close of the terminal area.

The active tab can be changed with keypress:
Next tab ctrl + .
Previous tab ctrl + ,

test2

Some comments:

I decided to avoid the shrinking of the tabs as they are added because it makes navigation and searches pretty difficult (titles are getting cropped and the size is too small). Looks like it's common to approach among IDEs to scroll tabs.

VSCode approach with scroll:
VSCode

Webstrom approach with arrows:
Screenshot 2022-01-25 at 16 37 40

E.g. browser scenario with too many tabs:
Screenshot 2022-01-25 at 16 31 39

Additional suggestions to add:

Drag and drop to change the order of the tabs?

UPDATE
Latest UX changes can be viewed below -> #4754 (comment)

@DmitriyNoa DmitriyNoa added area/ui enhancement New feature or request labels Jan 25, 2022
@DmitriyNoa DmitriyNoa force-pushed the improve-dock-tabs-ux branch from 665f37a to 994ae50 Compare January 25, 2022 16:33
@DmitriyNoa DmitriyNoa marked this pull request as ready for review January 25, 2022 16:34
@DmitriyNoa DmitriyNoa requested a review from a team as a code owner January 25, 2022 16:34
@DmitriyNoa DmitriyNoa requested review from jansav and Nokel81 and removed request for a team January 25, 2022 16:34
@Nokel81 Nokel81 added this to the 5.5.0 milestone Jan 25, 2022
@DmitriyNoa DmitriyNoa requested review from aleksfront and ixrock and removed request for ixrock January 25, 2022 17:38
ixrock
ixrock previously approved these changes Jan 27, 2022
Copy link
Contributor

@ixrock ixrock left a comment

Choose a reason for hiding this comment

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

LGTM

src/renderer/components/dock/dock.scss Outdated Show resolved Hide resolved
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
@DmitriyNoa DmitriyNoa force-pushed the improve-dock-tabs-ux branch from e12bf67 to 70c3d03 Compare January 31, 2022 08:52
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: DmitriyNoa <dmytro.zharkov@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Copy link
Collaborator

@Nokel81 Nokel81 left a comment

Choose a reason for hiding this comment

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

Looks great, though I have found some oddities:

  1. The new tab button is oddly moved to the right when there are no tabs:
    Screen Shot 2022-03-16 at 8 57 11 AM
  2. The close button covers text of the current tab:
    Screen Shot 2022-03-16 at 8 57 26 AM
  3. The current tab should be given more space so that you can still read it, even if the other tabs need to be shrunk:
    Screen Shot 2022-03-16 at 8 58 06 AM

src/renderer/hooks/useResizeObserver.ts Outdated Show resolved Hide resolved
@@ -20,6 +22,14 @@ export interface DockTabsProps {
}

export const DockTabs = ({ tabs, autoFocus, selectedTab, onChangeTab }: DockTabsProps) => {
const elem = useRef<HTMLDivElement>();
const minTabSize = useRef<number>(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this a ref and not state?

Copy link
Contributor

@aleksfront aleksfront Mar 18, 2022

Choose a reason for hiding this comment

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

Updating it only once and basically using it as instance variable - keeping across rerenders. https://reactjs.org/docs/hooks-faq.html#is-there-something-like-instance-variables

src/renderer/components/dock/dock-tabs.tsx Outdated Show resolved Hide resolved
src/renderer/components/dock/dock-tabs.tsx Outdated Show resolved Hide resolved
@jim-docker
Copy link
Contributor

jim-docker commented Mar 16, 2022

I'm seeing some odd behaviour, sometimes when I click on a tab (one in the middle, not clipped) the tabs shift but the tab I clicked on is not selected.

Screen.Recording.2022-03-16.at.11.32.27.AM.mov

Seems to only happen the first time you try to select a tab this way. I had to restart the app to reproduce.

@jim-docker
Copy link
Contributor

jim-docker commented Mar 16, 2022

Seems to open (and scroll if necessary) to the last tab when you close a tab:

Screen.Recording.2022-03-16.at.11.44.22.AM.mov

Better to open a neighbouring tab?

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
@aleksfront
Copy link
Contributor

@Nokel81 @jim-docker Fixed all addressed issues except one described below. Please review again.

dock.tabs.in.action.mov

single plus button
single tab
tabs as a whole

@aleksfront
Copy link
Contributor

Related this one

The current tab should be given more space so that you can still read it, even if the other tabs need to be shrunk:

I don't think it's a good decision since this will lead us to layout jumps since switching active tab will cause all sibling tabs to change their sizes instantly. Also, can't find similar behavior across other apps.

@aleksfront aleksfront requested a review from Nokel81 March 18, 2022 10:45
@Nokel81
Copy link
Collaborator

Nokel81 commented Mar 18, 2022

Okay I just know that in vscode the default tab size is such that for most file names it doesn't happen.

Nokel81
Nokel81 previously approved these changes Mar 18, 2022
@DmitriyNoa
Copy link
Contributor Author

LGTM

Copy link
Contributor

@jim-docker jim-docker left a comment

Choose a reason for hiding this comment

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

Got this when I clicked "Cancel" on an upgrade chart tab (doesn't happen on 5.4.3):

Screen Shot 2022-03-18 at 2 12 45 PM

src/renderer/components/dock/dock-tabs.tsx Outdated Show resolved Hide resolved
src/renderer/components/dock/dock.tsx Outdated Show resolved Hide resolved
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
@aleksfront
Copy link
Contributor

@jim-docker Created an issue with the bug you found. Happens in master too. #5050

@aleksfront aleksfront merged commit 9f6c3e2 into master Mar 21, 2022
@aleksfront aleksfront deleted the improve-dock-tabs-ux branch March 21, 2022 12:55
@Nokel81 Nokel81 mentioned this pull request May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Dock tabs UX
6 participants