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

fixed tab sizing: restore tab widths when the last tab is closed #183188

Merged
merged 3 commits into from May 23, 2023

Conversation

jacekkopecky
Copy link
Contributor

This is meant to address a forgotten case in #40290 that wasn't addressed in #181729: when the last tab is closed, there is no more reason to hold the tabs at fixed length. This behaviour is consistent with Chrome and Safari.

For illustration, here's a before and after:

2023-05-22 23 05 49

2023-05-22 23 05 18

(my cursor is huge and the recording software misplaced it, but it was right over the close icons)

cc @bpasero

Copy link
Member

@bpasero bpasero 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 this makes sense but what about the index that we pass in here [1], it is probably not the right value? Should we then remove it if we do not have it?

[1]

this.titleAreaControl.beforeCloseEditor(editor, index);

@jacekkopecky
Copy link
Contributor Author

@bpasero The beforeCloseEditor is modeled to mirror closeEditor with the same arguments.

It looks like neither of the two implementors of these methods, NoTabsTitleControl and TabsTitleControl actually use the index parameter. When I tried to use it, it turned out to be undefined anyway, so I used the editor parameter instead.

Further, the beforeCloseEditor method on TabsTitleControl is the only one to use the editor parameter. I can try to cut out all the unnecessary stuff if you'd like.

@bpasero
Copy link
Member

bpasero commented May 23, 2023

@jacekkopecky yeah I would think index parameter should be removed from beforeCloseEditor then.

jacekkopecky and others added 2 commits May 23, 2023 13:02
Nothing was using the index and beforeCloseEditor could
never get anything but undefined anyway.
@bpasero bpasero added this to the May 2023 milestone May 23, 2023
@bpasero bpasero enabled auto-merge (squash) May 23, 2023 12:18
@jacekkopecky
Copy link
Contributor Author

Thank you @bpasero . Nice catch with the isLast call, I missed that.

@bpasero bpasero merged commit fc063a5 into microsoft:main May 23, 2023
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants