Skip to content
Open
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
24 changes: 24 additions & 0 deletions doc-site/components/nve-heading.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
layout: component
---

<CodeExamplePreview>

```html
<nve-heading level="1">Dette er en overskrift</nve-heading>
```

</CodeExamplePreview>

## Eksempler

La til et par eksepler for å teste her:

<CodeExamplePreview>

```html
<nve-heading level="5">Dette er en h5 med default styling</nve-heading>
<nve-heading level="2" typographyType="headingMedium">Dette er en h2 med headingMedium typografi</nve-heading>
```

</CodeExamplePreview>
59 changes: 59 additions & 0 deletions src/components/nve-heading/nve-heading.component.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { LitElement } from 'lit';
import { unsafeStatic, html } from 'lit/static-html.js';
import { customElement, property } from 'lit/decorators.js';
import { INveComponent } from '@interfaces/NveComponent.interface';
import styles from './nve-heading.styles';

@customElement('nve-heading')
export default class NveHeading extends LitElement implements INveComponent {
@property({ type: String }) testId: string | undefined = undefined;
/** Heading level - Hvilket nivå overskriften skal ha */
@property({ type: Number, reflect: true }) level: 1 | 2 | 3 | 4 | 5 | 6 = 1;

/** Typografitype - Kan overstyre det som er standard typografi basert på nivå */
@property({ type: String, reflect: true }) typographyType?:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nå vet jo alle hva jeg mener om å innføre komponenter for tekst i designsystemet, men hvis vi ender opp med å få dette, stemmer jeg for å ikke bruke "typography" i begreper i koden. Jeg mener det er unødvendig komplisert og vanskelig å stave riktig. Hva med å kalle det size eller type?

Verdien trenger heller kanskje ikke hete nøyaktig det samme som css-variabelen? Jeg skjønner vi trenger å prefikse css-variablene med "heading" , men i denne komponenten blir det "smør på flesk" å skrive:

<nve-heading level="2" typographyType="headingLarge"/>Overskrift</nve-heading>

Dette synes jeg ser bedre ut:
<nve-heading level="2" size="large">Overskrift</nve-heading>

Men denne synes jeg er best:
<h2>Overskrift</h2>

Copy link
Contributor

Choose a reason for hiding this comment

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

Stemmer for kortere property-navn, men jeg synes ikke size er riktig her, siden vi peker til en token som inneholder font-size, font-weight, line-height og font-family. Size dekker ikke helt det vi faktisk styrer. Jeg tror enten variant eller type passer bedre her.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ja, første jeg tenkte på var å bare bruke "xlarge" | "large" ... istedenfor. Men kopierte litt props fra React-komponenten i første omgang bare for å få noe konkret kode. Ville også ha inn en nve-komponent for å teste med for å verifisere at Accessibility-treet ble korrekt og at den takler role=heading elementer som ligger i shadow-dom (det ser ut til å virke ok)

Tror at size vil fungere som attributt, siden det meste annet er felles (alle har samme font og vekt, tror alle ha samme line-height i prosent)

Copy link
Contributor

Choose a reason for hiding this comment

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

de bruker forskjellig line-height. jeg synes size hadde gitt mening hvis du satte opp en CSS-klasse som kun styrer font-size. Men siden det er font som endres her, blir size misvisende, tenker jeg.

Copy link
Contributor Author

@tomapedersen tomapedersen Mar 4, 2026

Choose a reason for hiding this comment

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

Ja, jeg regnet feil på line-height. Ser ut til at det korrekte er at alle har en utregnet line height på "font-size + ~6px"

xlarge har 40 / 46
large har 32 / 38.4
medium har 24 / 30
small har 20 / 26
xsmall har 18 / 23.4

Men absolutt verdt å diskutere noe annet enn size som navn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mulig at de ulike størrelsene også har ulik standard topp/bunn-marg, det har jeg ikke sjekket. Bør nok få avklart det med design og legge dem inn i css'en også

Copy link
Contributor

Choose a reason for hiding this comment

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

vi kan diskutere det, ja. når jeg tenker litt mer over det, indikerer selve token-navnet også størrelsen, så kanskje size er innafor her.

| 'headingXlarge'
| 'headingLarge'
| 'headingMedium'
| 'headingSmall';

static styles = [styles];

constructor() {
super();
}
protected getTagName() {
if (!this.level || this.level < 1 || this.level > 6 || isNaN(this.level)) {
return 'h1';
}
return `h${this.level}`;
}

protected getDefaultTypographyType() {
switch (this.level) {
case 1:
return 'headingXlarge';
case 2:
return 'headingLarge';
case 3:
return 'headingMedium';
default:
return 'headingSmall';
}
}

render() {
const tagName = this.getTagName();
return html`
<${unsafeStatic(tagName)} part="base" data-testid=${this.testId} class=${this.typographyType ? this.typographyType : this.getDefaultTypographyType()}>
<slot></slot>
</${unsafeStatic(tagName)}>
`;
}
}

declare global {
interface HTMLElementTagNameMap {
'nve-heading': NveHeading;
}
}
19 changes: 19 additions & 0 deletions src/components/nve-heading/nve-heading.styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { css } from 'lit';

export default css`
.headingXlarge {
font: var(--typography-heading-x-large);
}

.headingLarge {
font: var(--typography-heading-large);
}

.headingMedium {
font: var(--typography-heading-medium);
}

.headingSmall {
font: var(--typography-heading-small);
}
`;
20 changes: 20 additions & 0 deletions src/components/nve-heading/nve-heading.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { afterAll, describe, expect, it, } from 'vitest';
import { fixture, fixtureCleanup } from '@open-wc/testing';
import { html } from 'lit';
import NveHeading from './nve-heading.component';

if (!customElements.get('nve-heading')) {
customElements.define('nve-heading', NveHeading);
}

describe('nve-heading', () => {
afterAll(() => {
fixtureCleanup();
});

it('is attached to the DOM', async () => {
const el = await fixture<NveHeading>(html`<nve-heading></nve-heading>`);
expect(document.body.contains(el)).toBe(true);
});
});

1 change: 1 addition & 0 deletions src/nve-designsystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export { default as NveDialog } from './components/nve-dialog/nve-dialog.compone
export { default as NveDivider } from './components/nve-divider/nve-divider.component';
export { default as NveDrawer } from './components/nve-drawer/nve-drawer.component';
export { default as NveDropdown } from './components/nve-dropdown/nve-dropdown.component';
export { default as NveHeading } from './components/nve-heading/nve-heading.component';
export { default as NveIcon } from './components/nve-icon/nve-icon.component';
export { default as NveInput } from './components/nve-input/nve-input.component';
export { default as NveLabel } from './components/nve-label/nve-label.component';
Expand Down
Loading