Skip to content

Commit

Permalink
refactor: split dashboard spacing into gap and padding (#8205)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomivirkki authored Nov 22, 2024
1 parent 89ab3a8 commit ac9ee94
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 26 deletions.
13 changes: 7 additions & 6 deletions packages/dashboard/src/vaadin-dashboard-layout-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ export const DashboardLayoutMixin = (superClass) =>
#grid {
box-sizing: border-box;
--_vaadin-dashboard-default-spacing: 1rem;
--_vaadin-dashboard-spacing: max(
--_vaadin-dashboard-default-padding: 1rem;
--_vaadin-dashboard-padding: max(
0px,
var(--vaadin-dashboard-spacing, var(--_vaadin-dashboard-default-spacing))
var(--vaadin-dashboard-padding, var(--_vaadin-dashboard-default-padding))
);
padding: var(--_vaadin-dashboard-spacing);
padding: var(--_vaadin-dashboard-padding);
/* Default min and max column widths */
--_vaadin-dashboard-default-col-min-width: 25rem;
Expand Down Expand Up @@ -86,8 +86,9 @@ export const DashboardLayoutMixin = (superClass) =>
);
grid-auto-rows: var(--_vaadin-dashboard-row-height);
gap: var(--_vaadin-dashboard-spacing);
--_vaadin-dashboard-default-gap: 1rem;
--_vaadin-dashboard-gap: max(0px, var(--vaadin-dashboard-gap, var(--_vaadin-dashboard-default-gap)));
gap: var(--_vaadin-dashboard-gap);
}
::slotted(*) {
Expand Down
3 changes: 2 additions & 1 deletion packages/dashboard/src/vaadin-dashboard-layout.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import { DashboardLayoutMixin } from './vaadin-dashboard-layout-mixin.js';
* `--vaadin-dashboard-col-max-width` | maximum column width of the layout
* `--vaadin-dashboard-row-min-height` | minimum row height of the layout
* `--vaadin-dashboard-col-max-count` | maximum column count of the layout
* `--vaadin-dashboard-spacing` | spacing between child elements and space around its outer edges. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-gap` | gap between child elements. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-padding` | space around the dashboard's outer edges. Must be in length units (0 is not allowed, 0px is)
*
* The following state attributes are available for styling:
*
Expand Down
3 changes: 2 additions & 1 deletion packages/dashboard/src/vaadin-dashboard-layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import { DashboardLayoutMixin } from './vaadin-dashboard-layout-mixin.js';
* `--vaadin-dashboard-col-max-width` | maximum column width of the layout
* `--vaadin-dashboard-row-min-height` | minimum row height of the layout
* `--vaadin-dashboard-col-max-count` | maximum column count of the layout
* `--vaadin-dashboard-spacing` | spacing between child elements and space around its outer edges. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-gap` | gap between child elements. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-padding` | space around the dashboard's outer edges. Must be in length units (0 is not allowed, 0px is)
*
* The following state attributes are available for styling:
*
Expand Down
2 changes: 1 addition & 1 deletion packages/dashboard/src/vaadin-dashboard-section.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class DashboardSection extends DashboardItemMixin(ElementMixin(ThemableMixin(Pol
grid-template-columns: subgrid;
--_vaadin-dashboard-section-column: 1 / calc(var(--_vaadin-dashboard-effective-col-count) + 1);
grid-column: var(--_vaadin-dashboard-section-column) !important;
gap: var(--_vaadin-dashboard-spacing, 1rem);
gap: var(--_vaadin-dashboard-gap, 1rem);
/* Dashboard section header height */
--_vaadin-dashboard-section-header-height: minmax(0, auto);
grid-template-rows: var(--_vaadin-dashboard-section-header-height) repeat(
Expand Down
3 changes: 2 additions & 1 deletion packages/dashboard/src/vaadin-dashboard.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ export interface DashboardI18n {
* `--vaadin-dashboard-col-max-width` | maximum column width of the dashboard
* `--vaadin-dashboard-row-min-height` | minimum row height of the dashboard
* `--vaadin-dashboard-col-max-count` | maximum column count of the dashboard
* `--vaadin-dashboard-spacing` | spacing between child elements and space around its outer edges. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-gap` | gap between child elements. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-padding` | space around the dashboard's outer edges. Must be in length units (0 is not allowed, 0px is)
*
* The following state attributes are available for styling:
*
Expand Down
3 changes: 2 additions & 1 deletion packages/dashboard/src/vaadin-dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ import { WidgetResizeController } from './widget-resize-controller.js';
* `--vaadin-dashboard-col-max-width` | maximum column width of the dashboard
* `--vaadin-dashboard-row-min-height` | minimum row height of the dashboard
* `--vaadin-dashboard-col-max-count` | maximum column count of the dashboard
* `--vaadin-dashboard-spacing` | spacing between child elements and space around its outer edges. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-gap` | gap between child elements. Must be in length units (0 is not allowed, 0px is)
* `--vaadin-dashboard-padding` | space around the dashboard's outer edges. Must be in length units (0 is not allowed, 0px is)
*
* The following state attributes are available for styling:
*
Expand Down
42 changes: 33 additions & 9 deletions packages/dashboard/test/dashboard-layout.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,9 @@ describe('dashboard layout', () => {
});
});

describe('spacing', () => {
it('should have default spacing', async () => {
// Clear the spacing used in the tests
describe('gap', () => {
it('should have default gap', async () => {
// Clear the gap used in the tests
setSpacing(dashboard, undefined);
// Increase the width of the dashboard to fit two items, paddings and a gap
dashboard.style.width = `calc(${columnWidth}px * 2 + ${defaultSpacing * 3}px)`;
Expand All @@ -332,7 +332,7 @@ describe('dashboard layout', () => {
expect(item1Left - item0Right).to.eql(defaultSpacing);
});

it('should have custom spacing between items horizontally', async () => {
it('should have custom gap between items horizontally', async () => {
const customSpacing = 10;
setSpacing(dashboard, customSpacing);
// Increase the width of the dashboard to fit two items, paddings and a gap
Expand All @@ -345,7 +345,7 @@ describe('dashboard layout', () => {
expect(item1Left - item0Right).to.eql(customSpacing);
});

it('should have custom spacing between items vertically', async () => {
it('should have custom gap between items vertically', async () => {
const customSpacing = 10;
setSpacing(dashboard, customSpacing);
dashboard.style.width = `${columnWidth}px`;
Expand All @@ -358,6 +358,30 @@ describe('dashboard layout', () => {
});
});

describe('padding', () => {
it('should have default padding', async () => {
// Clear the padding used in the tests
setSpacing(dashboard, undefined);
await onceResized(dashboard);

const { left: itemLeft } = childElements[0].getBoundingClientRect();
const { left: dashbboardLeft } = dashboard.getBoundingClientRect();
// Expect the dashbaord to have default padding of 1rem
expect(itemLeft - dashbboardLeft).to.eql(defaultSpacing);
});

it('should have custom gap between items horizontally', async () => {
const customSpacing = 10;
setSpacing(dashboard, customSpacing);
await onceResized(dashboard);

const { left: itemLeft } = childElements[0].getBoundingClientRect();
const { left: dashbboardLeft } = dashboard.getBoundingClientRect();
// Expect the items to have a gap of 10px
expect(itemLeft - dashbboardLeft).to.eql(customSpacing);
});
});

describe('maximum column count', () => {
it('should not limit column count by default', async () => {
dashboard.style.width = `${columnWidth * 100}px`;
Expand Down Expand Up @@ -524,8 +548,8 @@ describe('dashboard layout', () => {
expect(newTitleHeight).to.eql(titleHeight);
});

describe('spacing', () => {
it('should have default spacing', async () => {
describe('gap', () => {
it('should have default gap', async () => {
// Clear the spacing used in the tests
setSpacing(dashboard, undefined);
// Increase the width of the dashboard to fit two items, paddings and a gap
Expand All @@ -538,7 +562,7 @@ describe('dashboard layout', () => {
expect(item3Left - item2Right).to.eql(defaultSpacing);
});

it('should have a custom spacing between items horizontally', async () => {
it('should have a custom gap between items horizontally', async () => {
const customSpacing = 10;
setSpacing(dashboard, customSpacing);
// Increase the width of the dashboard to fit two items, paddings and a gap
Expand All @@ -551,7 +575,7 @@ describe('dashboard layout', () => {
expect(item3Left - item2Right).to.eql(customSpacing);
});

it('should have a custom spacing between items vertically', async () => {
it('should have a custom gap between items vertically', async () => {
const customSpacing = 10;
setSpacing(dashboard, customSpacing);
dashboard.style.width = `${columnWidth}px`;
Expand Down
3 changes: 2 additions & 1 deletion packages/dashboard/test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ export function setRowspan(element: HTMLElement, rowspan?: number): void {
* Sets the spacing of the dashboard. This value adjusts the spacing between elements within the dashboard and the space around its outer edges.
*/
export function setSpacing(dashboard: HTMLElement, spacing?: number): void {
dashboard.style.setProperty('--vaadin-dashboard-spacing', spacing !== undefined ? `${spacing}px` : null);
dashboard.style.setProperty('--vaadin-dashboard-gap', spacing !== undefined ? `${spacing}px` : null);
dashboard.style.setProperty('--vaadin-dashboard-padding', spacing !== undefined ? `${spacing}px` : null);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { css, registerStyles } from '@vaadin/vaadin-themable-mixin/vaadin-themab

export const dashboardLayoutStyles = css`
#grid {
--_vaadin-dashboard-default-spacing: var(--lumo-space-l);
--_vaadin-dashboard-default-gap: var(--lumo-space-l);
--_vaadin-dashboard-default-padding: var(--lumo-space-l);
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { dashboardWidgetAndSection } from './vaadin-dashboard-widget-styles.js';

const section = css`
:host {
--_focus-ring-spacing-max-offset: calc(var(--_vaadin-dashboard-spacing) / 2);
--_focus-ring-spacing-max-offset: calc(min(var(--_vaadin-dashboard-gap), var(--_vaadin-dashboard-padding)) / 2);
}
:host([move-mode]) ::slotted(*) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ const dashboardWidgetAndSection = css`
/* default max value for the focus ring spacing offset. calc doesn't support unitless 0. */
/* stylelint-disable length-zero-no-unit */
--_focus-ring-spacing-max-offset: 0px;
/* Calculates the offset by which the focus ring should be shifted inwards based on a custom --vaadin-dashboard-spacing value.
Effectively keeps the focus ring visible if --vaadin-dashboard-spacing is set to 0px */
/* Calculates the offset by which the focus ring should be shifted inwards based on a custom --vaadin-dashboard-gap or --vaadin-dashboard-padding values.
Effectively keeps the focus ring visible if --vaadin-dashboard-gap or --vaadin-dashboard-padding is set to 0px */
--_focus-ring-spacing-offset: min(
max(calc(var(--_focus-ring-width) * -1), var(--_vaadin-dashboard-spacing) - var(--_focus-ring-width)),
max(
calc(var(--_focus-ring-width) * -1),
min(var(--_vaadin-dashboard-gap), var(--_vaadin-dashboard-padding)) - var(--_focus-ring-width)
),
var(--_focus-ring-spacing-max-offset, 0px)
);
outline: none;
Expand Down

0 comments on commit ac9ee94

Please sign in to comment.