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

Remote Host status bar entry has no max width #107451

Closed
jrieken opened this issue Sep 25, 2020 · 13 comments
Closed

Remote Host status bar entry has no max width #107451

jrieken opened this issue Sep 25, 2020 · 13 comments
Assignees
Labels
insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue remote Remote system operations issues workbench-status Status bar
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Sep 25, 2020

  • see attached screen capture
  • the remote host entry is too wide, we should crop it

Screenshot 2020-09-25 at 16 33 50

@bpasero bpasero assigned aeschli and unassigned bpasero Sep 25, 2020
@bpasero bpasero added workbench-status Status bar remote Remote system operations issues labels Sep 25, 2020
@bpasero
Copy link
Member

bpasero commented Sep 25, 2020

Yes I do agree. However more interesting is why the remote entry looks like that, it should not be so cryptic.

This is what I see:

image

Was this eventually going away?

@bpasero
Copy link
Member

bpasero commented Sep 25, 2020

Oh I get it, you did not use web but desktop. I wonder if maybe @legomushroom or @srivatsn could comment why desktop is rendered different from web. I suspect a different label formatter maybe?

@bpasero
Copy link
Member

bpasero commented Sep 25, 2020

@aeschli maybe when you detect the remote label to be very long you simply crop it in the middle adding a "..." or so?

@aeschli
Copy link
Contributor

aeschli commented Sep 25, 2020

@bpasero Not sure what you mean. You suggest we should crop it before setting to the statas bar? I think that should better be in the status bar in CSS.
It looks like the text-overflow: ellipsis; is there, but doesn't work together with display: flex

@srivatsn
Copy link

Originally the thinking was that in web, the URL has the context but in desktop it's not there but since GH codespaces names are too long, I agree this doesn't look good. @lostintangent @2percentsilk thoughts?

@2percentsilk
Copy link
Member

Yeah I'd agree that it's too long and a bit unsightly. Given the name of the codespace is also a part of the title bar, I don't think we need to have it fully duplicated at the bottom. Setting a max width and truncating the codespace name seems reasonable to me. For what exactly the max width would be I'd lean on @misolori :)

@miguelsolorio
Copy link
Contributor

I agree on keeping it shorter, just throwing a random number of 40 characters seems to look better. As @2percentsilk mentioned that the name is already repeated in other areas we don't need it as much here. It would also keep the width of the item ~275px.

image

@bpasero
Copy link
Member

bpasero commented Sep 27, 2020

@aeschli the reason I suggested a manual solution without CSS is that as far as I know there is no CSS rule to crop a label in the middle. But if we think that the label can be cropped at the end, I think a CSS rule could work.

Since each status item has a id, a CSS rule from the remoteIndicator such as:

#status\.host {
	max-width: 40px;
}

Should work. Maybe @misolori could advise for a good rule that works together with flex.

I am not sure I would go as far as to apply this rule to every item, not sure about the consequences for extensions. Related issue is #84258

@miguelsolorio
Copy link
Contributor

miguelsolorio commented Sep 28, 2020

I think in order for the ellipsis to work in flexbox the remote text needs to be wrapped in its own tag. We currently only wrap the codicon in a span. See example.

@aeschli
Copy link
Contributor

aeschli commented Nov 4, 2020

First of all, the remote extension should use a compact, human readable host name.
Still, I suggest to fix the status bar entry (which already has a max-width) so it gets the ... at the end. That way all status bar entries will profit and it's guaranteed that the maximum space is used. I personally don't think ... in the middle make anything better (really depends on the label), and it's not a good idea to to have RemoteStatusIndicator do any shorting as it will have to make assumptions on the current width of the status bar entry.

@aeschli aeschli assigned bpasero and unassigned aeschli Nov 4, 2020
@aeschli
Copy link
Contributor

aeschli commented Nov 4, 2020

Assigning to @bpasero to consider the change in the status bar as sketched by @misolori .

@bpasero bpasero added the feature-request Request for new features or functionality label Nov 4, 2020
@bpasero bpasero closed this as completed in 1dbff8b Nov 5, 2020
@bpasero bpasero added polish Cleanup and polish issue and removed feature-request Request for new features or functionality labels Nov 5, 2020
@bpasero
Copy link
Member

bpasero commented Nov 5, 2020

I ended up pushing a change to trim the remote label to max 40 chars as suggested by Miguel:

image

I don't to turn this into a general solution for all status bar entries because this is challenging, might not be the right solution for all status bar entries, and we have related issues to cover that in particular, namely #84258

@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
insiders-released Patch has been released in VS Code Insiders polish Cleanup and polish issue remote Remote system operations issues workbench-status Status bar
Projects
None yet
Development

No branches or pull requests

7 participants
@srivatsn @bpasero @jrieken @2percentsilk @aeschli @miguelsolorio and others