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

Revert hiding some notebook toolbars #161136

Merged
merged 6 commits into from Sep 20, 2022
Merged

Revert hiding some notebook toolbars #161136

merged 6 commits into from Sep 20, 2022

Conversation

roblourens
Copy link
Member

@roblourens roblourens commented Sep 16, 2022

We need to rerender the menu when thsi setting changes, because it determines which actionViewItem is used. cc @jrieken @rebornix

This reverts commit 517448a.

rebornix
rebornix previously approved these changes Sep 16, 2022
@jrieken
Copy link
Member

jrieken commented Sep 19, 2022

Sorry for the breakage and thanks for the fix

We need to rerender the menu when thsi setting changes,

Is that something that you can express with a config-context key? Each config-value is exposed as context key named config.<settings_name>

@roblourens
Copy link
Member Author

roblourens commented Sep 19, 2022

Hm, we already have two registrations in that menu, based on that config context key, so the label can change:

https://github.com/microsoft/vscode/blob/4c41daaa261c576e8c1e8b1ac1d47fe40820659c/src/vs/workbench/contrib/notebook/browser/controller/insertCellActions.ts#L224-L251

I guess I'm not sure why it wasn't being rerendered before

@roblourens
Copy link
Member Author

This is a real pain to debug, but I guess this is just because the setting is not officially registered, it's an undocumented setting. @rebornix should we just decide right now whether to officially register this setting (can still be "experimental") or delete it?

@roblourens roblourens changed the title Revert "use MenuWorkbenchToolBar in BetweenCellToolBar" Revert hiding some notebook toolbars Sep 20, 2022
@roblourens
Copy link
Member Author

In notebook standup we decided that supporting normal hiding doesn't really work in notebooks for most toolbars. They don't feel like normal menus, if we want to customize these things it should work differently. I reverted it for every toolbar except the title toolbar where it works quite well. Reverting the IW toolbar also fixed the issue we saw with the run button always showing inside a ... fyi @rebornix, there must be something unusual on that one.

@roblourens roblourens merged commit a829fbb into main Sep 20, 2022
@roblourens roblourens deleted the roblou/urban-bobcat branch September 20, 2022 01:15
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2022
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

4 participants