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.

Sign up for GitHub

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 issue where status bar can overflow without affecting notification beak #155649

Merged
merged 7 commits into from
Jul 22, 2022
Merged

Fixed issue where status bar can overflow without affecting notification beak #155649

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
895e23f
fixed status bar overflow while keeping beak intact
nirabhromakhal Jul 19, 2022
623f652
Merge remote-tracking branch 'origin/main'
nirabhromakhal Jul 19, 2022
ae5f722
using overflow: hidden while keeping beak intact
nirabhromakhal Jul 20, 2022
c797f53
made the notification bell absolute positioned
nirabhromakhal Jul 20, 2022
74b0dac
overflow: hidden but beak is relative to status bar item
nirabhromakhal Jul 21, 2022
19e1f5a
Merge branch 'microsoft:main' into main
nirabhromakhal Jul 21, 2022
9cb73a2
using has-beak class name to toggle the beak
nirabhromakhal Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/vs/workbench/browser/parts/statusbar/media/statusbarpart.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
height: 22px;
font-size: 12px;
display: flex;
overflow: visible;
overflow: hidden;
transition: background-color 0.35s ease-out;
}

Expand Down Expand Up @@ -47,16 +47,23 @@
vertical-align: top;
max-width: 40vw;
}

.monaco-workbench .part.statusbar > .items-container > .statusbar-item.has-beak {
position: relative;
}

.monaco-workbench .part.statusbar > .items-container > .statusbar-item.has-beak:before {
.monaco-workbench .part.statusbar > .items-container > .statusbar-item > .beak-div-show {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than having a beak-div-show, why not add a CSS class name to the beak container (status-item-beak-container) and then use the existing has-beak rule to make this work, e.g.:

.monaco-workbench .part.statusbar > .items-container > .statusbar-item.has-beak > .status-bar-beak-container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed a line-break (empty line) by mistake, should I commit again (with the line break)?

Copy link
Member

Choose a reason for hiding this comment

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

Sure.

content: '';
position: absolute;
left: 10px;
left: calc(50% - 5px); /* centering relative to parent */
top: -5px;
width: 10px;
height: 5px;
}

.monaco-workbench .part.statusbar > .items-container > .statusbar-item > .beak-div-show:before {
nirabhromakhal marked this conversation as resolved.
Show resolved Hide resolved
content: '';
position: fixed;
color: rgb(0, 122, 204);
border-bottom-width: 5px;
border-bottom-style: solid;
border-left: 5px solid transparent;
Expand All @@ -83,7 +90,7 @@
}

.monaco-workbench .part.statusbar > .items-container > .statusbar-item.right.last-visible-item {
padding-right: 7px; /* Add padding to the most right status bar item */
margin-right: 7px; /* Add padding to the most right status bar item */
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to change to margin here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The centering of the beak is done now based on the width of the status item.
Since the padding is included within the width for the bell, the beak doesn't show at the center for the bell but shows correctly for other status items, unless we change it to margin-right.

}

/* Tweak appearance for items with background to improve hover feedback */
Expand Down
10 changes: 10 additions & 0 deletions src/vs/workbench/browser/parts/statusbar/statusbarItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class StatusbarEntryItem extends Disposable {
private hover: ICustomHover | undefined = undefined;

readonly labelContainer: HTMLElement;
readonly beakContainer: HTMLElement;

get name(): string {
return assertIsDefined(this.entry).name;
Expand Down Expand Up @@ -73,6 +74,13 @@ export class StatusbarEntryItem extends Disposable {
// Add to parent
this.container.appendChild(this.labelContainer);

// Beak Container
this.beakContainer = document.createElement('div');
// this.beakContainer.classList.add('beak-div-show');
nirabhromakhal marked this conversation as resolved.
Show resolved Hide resolved

// Add to parent
this.container.appendChild(this.beakContainer);

this.update(entry);
}

Expand Down Expand Up @@ -145,8 +153,10 @@ export class StatusbarEntryItem extends Disposable {
if (!this.entry || entry.showBeak !== this.entry.showBeak) {
if (entry.showBeak) {
this.container.classList.add('has-beak');
this.beakContainer.classList.add('beak-div-show');
nirabhromakhal marked this conversation as resolved.
Show resolved Hide resolved
} else {
this.container.classList.remove('has-beak');
this.beakContainer.classList.remove('beak-div-show');
}
}

Expand Down