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

Jump to bottom

fix: Align the main headings on the data model page #13786

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

fix: Align the main headings on the data model page #13786

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fc539da
Align datamodel heading and schemaInspector tabs
ErlingHauan Oct 13, 2024
21258a0
Add separate heading and text content when no data model fields are s…
ErlingHauan Oct 13, 2024
a4917a2
Align TypesInspector heading with the other headings
ErlingHauan Oct 13, 2024
ac7fd59
Create border below headings for types, datamodel and properties
ErlingHauan Oct 13, 2024
98e99b6
Merge branch 'main' into fix/css-schema-inspector
ErlingHauan Oct 15, 2024
54d7553
Clean up
ErlingHauan Oct 16, 2024
1b2dc95
Fix selected type being sunk to bottom
ErlingHauan Oct 16, 2024
8687736
Update headings and add a test for no item selected
ErlingHauan Oct 17, 2024
97f9054
Fix most PR comments
ErlingHauan Oct 21, 2024
6353e90
Add label and vertical divider to toolbar
ErlingHauan Oct 21, 2024
e30ff38
Design decided, but not functionally implemented
ErlingHauan Oct 22, 2024
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
19 changes: 5 additions & 14 deletions frontend/packages/schema-editor/src/components/NodePanel/HeadingRow/HeadingRow.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,14 @@
.root {
--gap: var(--fds-spacing-1);
align-items: center;
display: flex;
padding: var(--gap) 0;
gap: var(--gap);
}

.heading {
display: contents;
align-items: center;
height: var(--subtoolbar-height);
gap: var(--fds-spacing-3);
border-bottom: 1px solid var(--fds-semantic-border-neutral-subtle);
}

.heading .headingButton {
border-bottom-left-radius: 0;
border-left-color: transparent;
border-left-style: solid;
border-left-width: var(--studio-treeitem-vertical-line-width);
border-top-left-radius: 0;
display: flex;
font: var(--fds-typography-paragraph-short-large);
min-height: var(--fds-sizing-12);
Copy link
Contributor

Choose a reason for hiding this comment

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

Disse egenskapene er her for å skape en likhet mellom rotelementet og underelementene når de er valgt (se bilder). Hva skjer når du tar dette bort?

image
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oi, det var ikke meningen. Bra du fanget det opp!

Ser at dette må være tilstede:

display: flex;
font: var(--fds-typography-paragraph-short-large);
border-left-width: var(--studio-treeitem-vertical-line-width);
border-top-left-radius: 0;
border-bottom-left-radius: 0;

Mens det virker som om dette ikke har noen funksjon:

border-left-color: transparent;
border-left-style: solid;

Bør være greit å ta vekk, eller?

Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg tror faktisk de to linjene også må være der. Når det står border-color: transparent, er det ofte for å hindre at ting flytter på seg når konturen blir borte eller dukker opp. Her skjer det når brukeren bytter mellom valgt / ikke valgt.

Eventuelt kunne vi valgt en helt annen fremgangsmåte her, f.eks. med background med linear-gradient eller med box-shadow. Da kan vi unngå denne litt uheldige effekten av å bruke border:
image

}

.root.selected .headingButton {
Expand Down
12 changes: 8 additions & 4 deletions frontend/packages/schema-editor/src/components/NodePanel/HeadingRow/HeadingRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import classes from './HeadingRow.module.css';
import { Heading } from '@digdir/designsystemet-react';
import { NodeIcon } from '../../NodeIcon';
import type { ReactNode } from 'react';
import React from 'react';
Expand All @@ -13,7 +12,12 @@ import {
SchemaModel,
} from '@altinn/schema-model';
import { useTranslation } from 'react-i18next';
import { StudioButton, StudioDeleteButton, StudioDropdownMenu } from '@studio/components';
import {
StudioButton,
StudioDeleteButton,
StudioDropdownMenu,
StudioHeading,
} from '@studio/components';
import {
BooleanIcon,
CombinationIcon,
Expand Down Expand Up @@ -44,7 +48,7 @@ export const HeadingRow = ({ schemaPointer }: HeadingRowProps) => {

return (
<div className={cn(classes.root, isSelected && classes.selected)}>
<Heading level={1} className={classes.heading}>
<StudioHeading level={1} className={classes.heading}>
<StudioButton
className={classes.headingButton}
color='second'
Expand All @@ -54,7 +58,7 @@ export const HeadingRow = ({ schemaPointer }: HeadingRowProps) => {
>
{title}
</StudioButton>
</Heading>
</StudioHeading>
{isValidParent && <AddNodeMenu schemaPointer={schemaPointer} />}
{!isDataModelRoot && <DeleteButton schemaPointer={schemaPointer} />}
</div>
Expand Down
8 changes: 4 additions & 4 deletions frontend/packages/schema-editor/src/components/NodePanel/NodePanel.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
.top {
box-shadow: var(--fds-shadow-small);
}

.backButton {
background-color: var(--fds-semantic-surface-action-subtle);
border: none;
Expand All @@ -12,3 +8,7 @@
.backButton:hover {
background-color: var(--fds-semantic-surface-action-subtle-hover);
}

.content {
padding-block: var(--fds-spacing-2);
}
8 changes: 4 additions & 4 deletions frontend/packages/schema-editor/src/components/NodePanel/NodePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ export const NodePanel = ({ schemaPointer }: NodePanelProps) => {

return (
<>
<div className={classes.top}>
{!isDataModelRoot && <BackButton />}
<HeadingRow schemaPointer={schemaPointer} />
<HeadingRow schemaPointer={schemaPointer} />
{!isDataModelRoot && <BackButton />}
Copy link
Contributor

Choose a reason for hiding this comment

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

Jeg er usikker på om vi burde plassere <BackButton> under overskriften. Blir det semantisk riktig? Dette er jo ikke innhold som er spesifikt for akkurat denne overskriften.

<div className={classes.content}>
{isNodeValidParent(node) && <SchemaTree schemaPointer={schemaPointer} />}
</div>
{isNodeValidParent(node) && <SchemaTree schemaPointer={schemaPointer} />}
</>
);
};
Expand Down
18 changes: 3 additions & 15 deletions frontend/packages/schema-editor/src/components/SchemaEditor/SchemaEditor.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
.editor {
display: flex;
flex-direction: column;
width: 100%;
background-color: white;
display: grid;
grid-template-rows: auto 1fr;
flex: 1;
min-height: 200px;
overflow-y: auto;
}
Expand All @@ -13,15 +13,3 @@
overflow-y: auto;
width: 100%;
}

.typeInfo {
display: flex;
flex-direction: row;
background-color: #022f51;
color: #ffffff;
align-items: center;
justify-content: center;
margin-left: auto;
margin-right: auto;
width: 100%;
}
12 changes: 4 additions & 8 deletions frontend/packages/schema-editor/src/components/SchemaEditor/SchemaEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { useUserQuery } from 'app-development/hooks/queries';
import { useStudioEnvironmentParams } from 'app-shared/hooks/useStudioEnvironmentParams';

export const SchemaEditor = () => {
const { schemaModel, selectedTypePointer, selectedUniquePointer } = useSchemaEditorAppContext();
const { schemaModel, selectedTypePointer } = useSchemaEditorAppContext();
const { org } = useStudioEnvironmentParams();
const { data: user } = useUserQuery();
const moveProperty = useMoveProperty();
Expand All @@ -35,21 +35,17 @@ export const SchemaEditor = () => {
orientation='horizontal'
localStorageContext={`datamodel:${user.id}:${org}`}
>
<StudioResizableLayout.Element minimumSize={100} maximumSize={280}>
<StudioResizableLayout.Element minimumSize={200} maximumSize={600}>
<aside className={classes.inspector}>
<TypesInspector schemaItems={definitions} />
</aside>
</StudioResizableLayout.Element>
<StudioResizableLayout.Element>
<StudioResizableLayout.Element minimumSize={350}>
<div className={classes.editor}>
<NodePanel schemaPointer={selectedType?.schemaPointer} />
</div>
</StudioResizableLayout.Element>
<StudioResizableLayout.Element
minimumSize={300}
collapsed={!selectedUniquePointer}
collapsedSize={180}
>
<StudioResizableLayout.Element minimumSize={300}>
<aside className={classes.inspector}>
<SchemaInspector />
</aside>
Expand Down
15 changes: 12 additions & 3 deletions frontend/packages/schema-editor/src/components/SchemaInspector/SchemaInspector.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,16 @@
height: 100%;
}

.noItem {
font-weight: 500;
margin: 18px;
.noSelectionHeadingContainer {
display: flex;
flex-direction: column;
justify-content: center;
padding-inline: var(--fds-spacing-6);
height: var(--subtoolbar-height);
border-bottom: 1px solid var(--fds-semantic-border-neutral-subtle);
}

.tabHeader {
height: var(--subtoolbar-height);
font-size: var(--fds-typography-heading-small);
}
24 changes: 18 additions & 6 deletions frontend/packages/schema-editor/src/components/SchemaInspector/SchemaInspector.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,19 @@ const renderSchemaInspector = (uiSchemaMap: UiSchemaNodes, selectedItem?: UiSche
describe('SchemaInspector', () => {
afterEach(jest.clearAllMocks);

it('Saves data model when entering text in textboxes', async () => {
it('displays a message when no node is selected', () => {
renderSchemaInspector(mockUiSchema);

const noSelectionHeader = screen.getByRole('heading', {
name: textMock('schema_editor.properties'),
});
const noSelectionParagraph = screen.getByText(textMock('schema_editor.no_item_selected'));

expect(noSelectionHeader).toBeInTheDocument();
expect(noSelectionParagraph).toBeInTheDocument();
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Fin test! Synes bare variablenavnet noSelectionHeader kan virke litt forvirrende, siden den bare sier "Egenskaper". Hva med propertiesHeader? Dette er småpirk.


it('saves data model when entering text in textboxes', async () => {
renderSchemaInspector(mockUiSchema, getMockSchemaByPath('#/$defs/Kommentar2000Restriksjon'));
const tablist = screen.getByRole('tablist');
expect(tablist).toBeDefined();
Expand All @@ -59,13 +71,13 @@ describe('SchemaInspector', () => {
expect(saveDataModel).toHaveBeenCalled();
});

test('renders no item if nothing is selected', () => {
it('renders no item if nothing is selected', () => {
renderSchemaInspector(mockUiSchema);
const textboxes = screen.queryAllByRole('textbox');
expect(textboxes).toHaveLength(0);
});

it('Saves data model correctly when changing restriction value', async () => {
it('saves data model correctly when changing restriction value', async () => {
const schemaPointer = '#/$defs/Kommentar2000Restriksjon';

renderSchemaInspector(mockUiSchema, getMockSchemaByPath(schemaPointer));
Expand Down Expand Up @@ -93,7 +105,7 @@ describe('SchemaInspector', () => {
expect(updatedNode.restrictions.minLength).toEqual(parseInt(minLength));
});

test('Adds new object field when pressing the enter key', async () => {
it('adds new object field when pressing the enter key', async () => {
const parentNodePointer = '#/properties/test';
const childNodePointer = '#/properties/test/properties/abc';
const rootNode: FieldNode = {
Expand Down Expand Up @@ -125,7 +137,7 @@ describe('SchemaInspector', () => {
});
});

test('Adds new valid value field when pressing the enter key', async () => {
it('adds new valid value field when pressing the enter key', async () => {
const itemPointer = '#/properties/test';
const enumValue = 'valid value';
const rootNode: FieldNode = {
Expand Down Expand Up @@ -155,7 +167,7 @@ describe('SchemaInspector', () => {
expect(saveDataModel).not.toHaveBeenCalled();
});

it('Does not display the fields tab when the selected item is a combination', async () => {
it('does not display the fields tab when the selected item is a combination', async () => {
const itemPointer = '#/properties/testcombination';
const rootNode: FieldNode = {
...rootNodeMock,
Expand Down
42 changes: 28 additions & 14 deletions frontend/packages/schema-editor/src/components/SchemaInspector/SchemaInspector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,49 @@
import type { ReactElement } from 'react';
import React from 'react';
import { Alert, Tabs } from '@digdir/designsystemet-react';
import type { UiSchemaNode } from '@altinn/schema-model';
import { isField, isObject } from '@altinn/schema-model';
import { ItemPropertiesTab } from './ItemPropertiesTab';
import { ItemFieldsTab } from './ItemFieldsTab';
import classes from './SchemaInspector.module.css';
import { Divider } from 'app-shared/primitives';
import { useTranslation } from 'react-i18next';
import { useSchemaEditorAppContext } from '../../hooks/useSchemaEditorAppContext';
import { useSavableSchemaModel } from '../../hooks/useSavableSchemaModel';
import { StudioCenter, StudioHeading, StudioParagraph } from '@studio/components';

export const SchemaInspector = () => {
export const SchemaInspector = (): ReactElement => {
const { t } = useTranslation();
const { selectedUniquePointer } = useSchemaEditorAppContext();
const savableModel = useSavableSchemaModel();
const selectedItem: UiSchemaNode = selectedUniquePointer
? savableModel.getNodeByUniquePointer(selectedUniquePointer)
: undefined;
const shouldDisplayFieldsTab = selectedItem && isField(selectedItem) && isObject(selectedItem);

if (!selectedUniquePointer) {
if (!selectedItem) {
return (
<div>
<p className={classes.noItem}>{t('schema_editor.no_item_selected')}</p>
<Divider />
</div>
<>
<div className={classes.noSelectionHeadingContainer}>
<StudioHeading level={2} size='2xs'>
{t('schema_editor.properties')}
</StudioHeading>
</div>
<StudioCenter>
<StudioParagraph size='sm'>{t('schema_editor.no_item_selected')}</StudioParagraph>
</StudioCenter>
</>
);
}

const selectedItem: UiSchemaNode = savableModel.getNodeByUniquePointer(selectedUniquePointer);
const shouldDisplayFieldsTab = isField(selectedItem) && isObject(selectedItem);

return (
<Tabs defaultValue={t('schema_editor.properties')} className={classes.root}>
<Tabs defaultValue={t('schema_editor.properties')}>
<Tabs.List>
<Tabs.Tab value={t('schema_editor.properties')}>{t('schema_editor.properties')}</Tabs.Tab>
<Tabs.Tab value={t('schema_editor.fields')}>{t('schema_editor.fields')}</Tabs.Tab>
<Tabs.Tab value={t('schema_editor.properties')} className={classes.tabHeader}>
{t('schema_editor.properties')}
</Tabs.Tab>
<Tabs.Tab value={t('schema_editor.fields')} className={classes.tabHeader}>
{t('schema_editor.fields')}
</Tabs.Tab>
</Tabs.List>
<Tabs.Content value={t('schema_editor.properties')}>
<ItemPropertiesTab selectedItem={selectedItem} />
Expand All @@ -41,7 +53,9 @@ export const SchemaInspector = () => {
<ItemFieldsTab selectedItem={selectedItem} />
</Tabs.Content>
) : (
<Alert severity='info'>{t('app_data_modelling.fields_information')}</Alert>
<Tabs.Content value={t('schema_editor.fields')}>
<Alert severity='info'>{t('app_data_modelling.fields_information')}</Alert>
</Tabs.Content>
Copy link
Contributor

Choose a reason for hiding this comment

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

Fint at du fikk plassert denne i en <Tabs.Content>! Kanskje du også kan flytte inn hele shouldDisplayFieldsTab-sjekken, siden det ikke er noen forskjell på de to <Tabs.Content>-ene?

)}
</Tabs>
);
Expand Down
43 changes: 13 additions & 30 deletions frontend/packages/schema-editor/src/components/TypesInspector/TypesInspector.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,20 @@
.root {
box-sizing: border-box;
padding: 24px 24px;
min-height: 100%;
background: rgba(224, 224, 224, 0.3);
border-right: 1px solid var(--fds-semantic-border-neutral-subtle);
}

.types {
.headingContainer {
display: flex;
flex-direction: column;
align-items: stretch;
gap: 8px;
}

.addRow {
display: grid;
grid-template-columns: repeat(3, 1fr);
justify-content: space-between;
align-items: center;
height: var(--subtoolbar-height);
padding-inline: var(--fds-spacing-6);
border-bottom: 1px solid var(--fds-semantic-border-neutral-subtle);
}

.addRowText {
grid-column: 1 / 3;
font-size: 18px;
.addTypeButton {
color: var(--fds-semantic-surface-neutral-inverted);
}

.addRowButton {
grid-column: 3;
margin-left: auto;
margin-right: 0;
color: #000000;
}

.noItem {
font-weight: 500;
margin: 18px;
.typesList {
display: flex;
flex-direction: column;
padding-block: var(--fds-spacing-5);
padding-inline: var(--fds-spacing-6);
gap: var(--fds-spacing-3);
}
Loading