Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions tensorbored/webapp/metrics/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ export const metricsTagGroupExpansionStateLoaded = createAction(
props<{expandedGroups: Array<[string, boolean]>}>()
);

export const metricsSuperimposedSectionExpansionChanged = createAction(
'[Metrics] Superimposed Section Expansion Changed'
);

export const cardFullWidthStateLoaded = createAction(
'[Metrics] Card Full Width State Loaded From Storage',
props<{fullWidthCardIds: string[]; fullWidthSuperimposedCardIds: string[]}>()
Expand Down
24 changes: 13 additions & 11 deletions tensorbored/webapp/metrics/effects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ import {
getTagSymlogLinearThresholds,
getSuperimposedCardsWithMetadata,
getMetricsTagGroupExpansionMap,
getMetricsSuperimposedSectionExpanded,
getCardStateMap,
getFullWidthSuperimposedCards,
} from '../store';
Expand Down Expand Up @@ -875,22 +876,23 @@ export class MetricsEffects implements OnInitEffects {
this.persistTagGroupExpansion$ = this.actions$.pipe(
ofType(
actions.metricsTagGroupExpansionChanged,
actions.metricsTagMetadataLoaded
actions.metricsTagMetadataLoaded,
actions.metricsSuperimposedSectionExpansionChanged
),
debounceTime(200),
withLatestFrom(this.store.select(getMetricsTagGroupExpansionMap)),
tap(([, expansionMap]) => {
withLatestFrom(
this.store.select(getMetricsTagGroupExpansionMap),
this.store.select(getMetricsSuperimposedSectionExpanded)
),
tap(([, expansionMap, superimposedExpanded]) => {
const entries: Array<[string, boolean]> = Array.from(
expansionMap.entries()
);
if (entries.length > 0) {
window.localStorage.setItem(
TAG_GROUP_EXPANSION_STORAGE_KEY,
JSON.stringify({version: 1, groups: entries})
);
} else {
window.localStorage.removeItem(TAG_GROUP_EXPANSION_STORAGE_KEY);
}
entries.push(['__superimposed__', superimposedExpanded]);
window.localStorage.setItem(
TAG_GROUP_EXPANSION_STORAGE_KEY,
JSON.stringify({version: 1, groups: entries})
);
window.dispatchEvent(new CustomEvent('tb-tag-group-expansion-changed'));
})
);
Expand Down
18 changes: 16 additions & 2 deletions tensorbored/webapp/metrics/store/metrics_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ const {initialState, reducers: namespaceContextedReducer} =
superimposedCardMetadataMap: {},
superimposedCardList: [],
fullWidthSuperimposedCards: new Set<string>(),
superimposedSectionExpanded: true,
},
{
isSettingsPaneOpen: true,
Expand Down Expand Up @@ -1280,8 +1281,21 @@ const reducer = createReducer(
return {...state, tagGroupExpanded};
}),
on(actions.metricsTagGroupExpansionStateLoaded, (state, {expandedGroups}) => {
const tagGroupExpanded = new Map<string, boolean>(expandedGroups);
return {...state, tagGroupExpanded};
const superimposedEntry = expandedGroups.find(
([key]) => key === '__superimposed__'
);
const tagGroupExpanded = new Map<string, boolean>(
expandedGroups.filter(([key]) => key !== '__superimposed__')
);
const superimposedSectionExpanded =
superimposedEntry !== undefined ? superimposedEntry[1] : true;
return {...state, tagGroupExpanded, superimposedSectionExpanded};
}),
on(actions.metricsSuperimposedSectionExpansionChanged, (state) => {
return {
...state,
superimposedSectionExpanded: !state.superimposedSectionExpanded,
};
}),
on(
actions.cardFullWidthStateLoaded,
Expand Down
5 changes: 5 additions & 0 deletions tensorbored/webapp/metrics/store/metrics_selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,11 @@ export const getMetricsTagGroupExpansionMap = createSelector(
}
);

export const getMetricsSuperimposedSectionExpanded = createSelector(
selectMetricsState,
(state: MetricsState): boolean => state.superimposedSectionExpanded
);

export const getMetricsLinkedTimeEnabled = createSelector(
selectMetricsState,
(state: MetricsState): boolean => {
Expand Down
7 changes: 7 additions & 0 deletions tensorbored/webapp/metrics/store/metrics_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,13 @@ export interface MetricsNamespacedState {
* Set of superimposed card IDs that are displayed at full width.
*/
fullWidthSuperimposedCards: Set<SuperimposedCardId>;

/**
* Whether the Superimposed section header is expanded (cards visible).
* Defaults to true. Persisted in the same localStorage key as tagGroupExpanded,
* using the reserved key '__superimposed__'.
*/
superimposedSectionExpanded: boolean;
}

export interface MetricsSettings {
Expand Down
1 change: 1 addition & 0 deletions tensorbored/webapp/metrics/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ function buildBlankState(): MetricsState {
superimposedCardMetadataMap: {},
superimposedCardList: [],
fullWidthSuperimposedCards: new Set<string>(),
superimposedSectionExpanded: true,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,21 @@ limitations under the License.

.group-toolbar {
@include tb-theme-background-prop(background-color, background);
@include tb-theme-foreground-prop(border-bottom, border, 1px solid);
@include tb-theme-foreground-prop(color, text);
align-items: center;
background-color: #fff;
border: 0;
@include tb-theme-foreground-prop(border-bottom, border, 1px solid);
cursor: pointer;
display: flex;
flex: none;
font: inherit;
height: 42px;
margin-bottom: -1px;
padding: 0 16px;
position: sticky;
top: 0;
width: 100%;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Button overflows container due to missing box-sizing

Medium Severity

The .group-toolbar was converted from a <div> to a <button>, and width: 100% was added along with padding: 0 16px. There is no global box-sizing: border-box reset in this project (confirmed by searching the codebase and global styles). Under the default content-box model, width: 100% sets the content width to 100% of the parent, then padding adds 32px on top, causing the button to overflow its container horizontally. The original <div> didn't need explicit width because block elements auto-fill their container while accounting for padding. A box-sizing: border-box declaration is needed on this element.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 217c88b. Configure here.

z-index: 1;
box-shadow: 0px 2px 4px 0px rgba(#000, 15%);
@include tb-dark-theme {
Expand All @@ -40,8 +45,10 @@ limitations under the License.
.left-items {
align-items: center;
display: flex;
flex-grow: 1;
gap: 10px;
overflow: hidden;
text-align: left;

mat-icon {
color: #673ab7;
Expand Down Expand Up @@ -70,6 +77,10 @@ limitations under the License.
}
}

.expand-group-icon {
@include tb-theme-foreground-prop(color, secondary-text);
}

.superimposed-cards-grid {
display: grid;
grid-template-columns: repeat(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ limitations under the License.
import {
ChangeDetectionStrategy,
Component,
EventEmitter,
Input,
OnChanges,
Output,
SimpleChanges,
} from '@angular/core';
import {Store} from '@ngrx/store';
Expand All @@ -35,7 +37,14 @@ const MAX_CARD_MIN_WIDTH_IN_PX = 735;
selector: 'superimposed-cards-view-component',
template: `
<ng-container *ngIf="superimposedCards.length > 0">
<div class="group-toolbar">
<button
class="group-toolbar"
i18n-aria-label="
A button that allows user to expand the superimposed section.
"
aria-label="Expand superimposed section"
(click)="expansionToggled.emit()"
>
<div class="left-items">
<mat-icon svgIcon="group_work_24px"></mat-icon>
<span class="group-text">
Expand All @@ -47,8 +56,18 @@ const MAX_CARD_MIN_WIDTH_IN_PX = 735;
>
</span>
</div>
</div>
<span class="expand-group-icon">
<mat-icon
*ngIf="isExpanded; else expandMore"
svgIcon="expand_less_24px"
></mat-icon>
<ng-template #expandMore>
<mat-icon svgIcon="expand_more_24px"></mat-icon>
</ng-template>
</span>
</button>
<div
*ngIf="isExpanded"
class="superimposed-cards-grid"
[style.grid-template-columns]="gridTemplateColumn"
>
Expand Down Expand Up @@ -76,6 +95,8 @@ export class SuperimposedCardsViewComponent implements OnChanges {
@Input() cardObserver!: CardObserver;
@Input() superimposedCards: SuperimposedCardMetadata[] = [];
@Input() cardMinWidth: number | null = null;
@Input() isExpanded: boolean = true;
@Output() expansionToggled = new EventEmitter<void>();

gridTemplateColumn = '';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ import {Observable} from 'rxjs';
import {startWith} from 'rxjs/operators';
import {State} from '../../../app_state';
import {getMetricsCardMinWidth} from '../../../selectors';
import {getSuperimposedCardsWithMetadata} from '../../store';
import {
getSuperimposedCardsWithMetadata,
getMetricsSuperimposedSectionExpanded,
} from '../../store';
import {metricsSuperimposedSectionExpansionChanged} from '../../actions';
import {SuperimposedCardMetadata} from '../../types';
import {CardObserver} from '../card_renderer/card_lazy_loader';

Expand All @@ -30,6 +34,8 @@ import {CardObserver} from '../card_renderer/card_lazy_loader';
[superimposedCards]="superimposedCards$ | async"
[cardObserver]="cardObserver"
[cardMinWidth]="cardMinWidth$ | async"
[isExpanded]="isExpanded$ | async"
(expansionToggled)="onExpansionToggled()"
></superimposed-cards-view-component>
`,
changeDetection: ChangeDetectionStrategy.OnPush,
Expand All @@ -39,11 +45,17 @@ export class SuperimposedCardsViewContainer {

readonly superimposedCards$: Observable<SuperimposedCardMetadata[]>;
readonly cardMinWidth$: Observable<number | null>;
readonly isExpanded$: Observable<boolean>;

constructor(private readonly store: Store<State>) {
this.superimposedCards$ = this.store
.select(getSuperimposedCardsWithMetadata)
.pipe(startWith([]));
this.cardMinWidth$ = this.store.select(getMetricsCardMinWidth);
this.isExpanded$ = this.store.select(getMetricsSuperimposedSectionExpanded);
}

onExpansionToggled() {
this.store.dispatch(metricsSuperimposedSectionExpansionChanged());
}
}
Loading