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

VS Code for the Web: title area shows up in pre-existing sessions #161948

Closed
isidorn opened this issue Sep 27, 2022 · 22 comments
Closed

VS Code for the Web: title area shows up in pre-existing sessions #161948

isidorn opened this issue Sep 27, 2022 · 22 comments
Assignees
Labels
command-center titlebar VS Code main title bar issues under-discussion Issue is under discussion for relevance, priority, approach web Issues related to running VSCode in the web
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Sep 27, 2022

https://insiders.vscode.dev

Title area is rendered? I personally find this unexpected, since the title area does not bring that much value and occupies precious screen real estate. On first open we also show a blue banner, the banner with the title area give a bad first user impression. We should aim for a light experience imho

Screenshot 2022-09-27 at 11 26 34

@isidorn isidorn added the regression Something that used to work is now broken label Sep 27, 2022
@isidorn
Copy link
Contributor Author

isidorn commented Sep 27, 2022

@joaomoreno says
it's very hard to reproduce since the steps are:

  1. open a browser session in which you have previously opened insiders.vscode.dev
  2. open insiders.vscode.dev

🐛 Titlebar area appears

⚠️ As soon as you clear cache or reset user data, the state seems gone.

fyi @jrieken @sandy081

@joaomoreno joaomoreno changed the title vscode.dev title area rendered? VS Code for the Web: title area shows up in pre-existing sessions Sep 27, 2022
@bpasero bpasero assigned jrieken and sbatten and unassigned bpasero Sep 27, 2022
@bpasero bpasero added command-center web Issues related to running VSCode in the web and removed regression Something that used to work is now broken labels Sep 27, 2022
@bpasero
Copy link
Member

bpasero commented Sep 27, 2022

You have command center enabled.

@bpasero bpasero added the titlebar VS Code main title bar issues label Sep 27, 2022
@joaomoreno
Copy link
Member

Yes, that is the change:

image

@joaomoreno
Copy link
Member

This explains why the bug goes away in a new session:

image

@joaomoreno
Copy link
Member

@jrieken, @digitarald:

  • As a specific fix, I would disable this experiment on the web
  • As a general fix, I would make sure that window.commandCenter has a default value of false on the web, even if we decided to switch to true on desktop

@jrieken
Copy link
Member

jrieken commented Sep 27, 2022

As a specific fix, I would disable this experiment on the web

I take this as your personal feedback and you have a way to disable this. However, I don't see why this should be specifically disabled in web. This is more or less exactly how it looks in mac desktop and we have very similar looks in web when using the window.menuBarVisibility options, using visible or classic.

@jrieken jrieken added the under-discussion Issue is under discussion for relevance, priority, approach label Sep 27, 2022
@jrieken
Copy link
Member

jrieken commented Sep 27, 2022

⚠️ As soon as you clear cache or reset user data, the state seems gone.

That's the power of experiments and there is no (good) UI to know what experiments you are exposed to (except for F1 > Report Issue). For those that don't like surprises there is workbench.enableExperiments

@joaomoreno
Copy link
Member

joaomoreno commented Sep 29, 2022

This is how https://vscode.dev looks to me now:

image

Again, on the web, I have a hard time justifying the appearance of the title bar for the sake of having a control center, and I argue we'd be making a mistake enabling this by default. Again, I'm talking about on the web.

I'm curious to know what other people think. Please chime in @digitarald.

@jrieken
Copy link
Member

jrieken commented Sep 29, 2022

Again, I'm talking about on the web.

I think we all understand this is on the web. Tho, I don't understand what your point is? You are OK with two sidebars but not two titlebars?

@isidorn
Copy link
Contributor Author

isidorn commented Sep 29, 2022

As in my first comment I believe web should be a light experience and two titlebars do not help here. It looks very alien to the eye of a user in a browser. I strongly suggest we fix this for the web.

@jrieken
Copy link
Member

jrieken commented Sep 29, 2022

I suggest we take this as datapoints and continue with the experiment - also for web. If you manually disable/opt-out-of the CC it is automatically tracked in the corresponding dashboard. That helps to get reliable numbers that we can base our decisions on. Thanks for understanding.

@bpasero
Copy link
Member

bpasero commented Sep 29, 2022

My 2 cents: I immediately hide the titlebar or any menu bar in web when I am browsing like a normal web site, but where I would want to see the titlebar is when I use VSCode from a PWA. Thus, you maybe in web but you want a custom title bar if PWA is enabled (this is something we can detect). Related issue: #140694

@joaomoreno
Copy link
Member

joaomoreno commented Sep 29, 2022

@jrieken My point is: I love the command center. But the title bar does not look good on a web browser at all. It looks odd, out of place and ruins the current cookies banner (arguably a current bug of the cookies banner). And if we don't intend on showing the titlebar on the web by default until we address those issues, why are we forcing these users to go through this experiment? I argue the common Joe doesn't even realize he can hide the titlebar.

@digitarald
Copy link
Contributor

@joaomoreno thank you, that screenshot clarified it for me. In the past we decided to not run experiments on vscode.dev (we filter the data out from scorecards, but still seem to apply them). cc @devinvalenciano @lramos15

It seems like a side effect that enabling the command palette also triggers the title bar. Enabling layout controls does not trigger the title. Not sure how we should make that consistent.

@joaomoreno
Copy link
Member

Discussed in ZH Standup. A good low hanging fix would be to only add the experimental tag to this setting when not in Web.

Also, follow up issue: #162424

@jrieken
Copy link
Member

jrieken commented Sep 30, 2022

@alexdima and I discussed this and we concluded that the underlying issue is more nuanced. On certain platforms, like web, enabling the command center does enable the custom title bar. That means the CC-experiment actually does two things: it enables the CC and the custom title bar. This lowers confidence in the outcomes and we should look into improving things

The current behaviour (CC enables custom title bar) came in with #151892. Before tanking the experiment I will investigate to disable that behaviour - I am certain it came from a time where the custom menu (in the title bar) wasn't responding to layout changes properly

@sbatten
Copy link
Member

sbatten commented Oct 5, 2022

The fix to decouple visibility of the title bar from the command center introduces the issue that the user has no way of using the command center even if they have manually enabled it without messing with menu bar visibility which they won't know about. The change I made here to have them coupled was to make the "best guess" based on what the user has for settings. Why enable the command center if you don't want to see it? I understand we are running an experiment so it's not true that they enabled it themselves but now enabling it on web has zero effect in non-pwa form.

We either need to have these "smarts" back, or we need to introduce yet another setting for titlebar visibility.

@sbatten
Copy link
Member

sbatten commented Oct 18, 2022

Some options for settings, @jrieken @joaomoreno please let me know what you think about these or if you have additional ideas.

--- Components Based: Title Bar Settings talk about contents of the title bar ---

Menu Bar Visibility
classic (not visible in fullscreen)
visible (visible in fullscreen)
toggle (will push open title bar if alt is pressed)
hidden (not shown at all)
compact (in activity bar)

Title Bar Visibility
always (show no matter what)
contents (show if any contents (menu bar, cc, layout controls) are enabled)
windowControlsOnly (shows only if window controls are presented)

  • Introducing a setting is tricky because titlebar visibility is also coupled with title bar style.

--- Title Bar Visibility has the final say ---

Menu Bar Visibility
classic (match title bar visibility)
visible (match title bar visibility, force in fullscreen)
toggle (will push open title bar if alt is pressed)
hidden (not shown at all)
compact (in activity bar)

Title Bar Visibility
always (shows no matter what)
classic (shows when not in fullscreen)
windowControlsOnly (shows only if window controls are presented)

  • This is tricky because classic and visible have different behaviors for native and custom title bars on Windows/Linux making this option confusing

--- No New Setting ---

Title Bar behaves like "contents (show if any contents are enabled)". i.e. if command center, menu bar, or layout controls are shown, the title bar should be visible. Right click > hide can turn off these contents to dismiss the title bar.

  • This is essentially what we had before which caused concerns from the team.

@joaomoreno
Copy link
Member

--- No New Setting ---

This is the best option.

The concerns above are:

  • The cookies header sits between the titlebar and the workbench. This is already addressed: Web: Welcome banner appears below titlebar #162424
  • Having the command center in the title bar generally looks great in Desktop and PWA, because a title bar makes sense in those environments. On the web, the title should just be a container for other components, not so much a title bar per se, given the browser already has a title bar. So, we should make it look less like a title bar, and more like an integrated container for other components. One low hanging fruit: remove the VS Code icon from the left. Slack does this with the hamburger menu:

image

image

@jrieken
Copy link
Member

jrieken commented Oct 20, 2022

I do like the combination of burger and CC but I also know that the ergonomics of the burger isn't as good as classic menu. (cc @dbaeumer). So, please excuse me, I don't want to derail this discussion nor make things more complicated but would we consider a two line title bar for certain cases, esp with classic menus.

For web there is some feedback that classic menu is preferred and for desktop (linux, window) it's the default. When enabling the CC in these cases there are a couple of issues:

  • crowded looks
  • CC isn't center aligned and fights the menu for space
  • title area lacks drag regions (paper cut)

This makes me wonder if CC and classic menu should render below each other, similar to O365 app (see below)

Screenshot 2022-10-20 at 17 58 03

Find my super profession mock-up below 👇. Again, this would only apply when enabling the CC and the classic menu at the same time, e.g windows, linux and some web users. It might require some platform specific (none sync'ed?) settings to enable the burger for web but not for windows.

2title

@dbaeumer
Copy link
Member

Usually, I have enough space for both the command center and the menu bar. But that might be me having a wide monitor :-)

@jrieken jrieken modified the milestones: October 2022, November 2022 Oct 24, 2022
sbatten added a commit that referenced this issue Oct 31, 2022
refs VS Code for the Web: title area shows up in pre-existing sessions #161948
sbatten added a commit that referenced this issue Nov 8, 2022
refs VS Code for the Web: title area shows up in pre-existing sessions #161948
@sbatten
Copy link
Member

sbatten commented Nov 23, 2022

The title bar is now conditional based on the contents and the banner issue is fixed. Web is also no longer pushing an app icon into the mix. Also the menubar converts to a hamburger when size is small. For this reason, I think this issue is resolved. If we want to explore pushing the menu down, I suggest this be a separate feature request.

@sbatten sbatten closed this as completed Nov 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
command-center titlebar VS Code main title bar issues under-discussion Issue is under discussion for relevance, priority, approach web Issues related to running VSCode in the web
Projects
None yet
Development

No branches or pull requests

7 participants