Skip to content

Add new text styles - #122

Merged
barry-observation merged 3 commits into
developfrom
feature/new-static-text-styles
Sep 14, 2026
Merged

barry-observation merged 3 commits into
developfrom
feature/new-static-text-styles

Conversation

@barry-observation

Copy link
Copy Markdown
Contributor

@barry-observation
barry-observation force-pushed the feature/new-static-text-styles branch from 5c4f58e to e992062 Compare September 10, 2026 14:51
@barry-observation
barry-observation force-pushed the feature/new-static-text-styles branch from e992062 to fe480f5 Compare September 10, 2026 15:06
Comment thread src/theme/tokens/text.ts
fontWeight: 'bold',
color: theme.color.text.system.strong,
},
speciesLocalNameCompact: {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Als je binnen de factory method dit definieert:

  const speciesLocalName = {
    fontFamily: 'Ubuntu',
    fontStyle: 'normal',
    fontSize: 16,
    lineHeight: 24,
    fontWeight: 'bold',
    color: theme.color.text.system.strong,
  } satisfies Text['speciesLocalName']

dan kun je hier dit doen:

    speciesLocalName,
    speciesLocalNameCompact: {
      ...speciesLocalName,
      lineHeight: 22,
    },

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍 aangepast

Comment thread src/theme/tokens/text.ts Outdated
fontFamily: 'Ubuntu',
fontStyle: 'normal',
fontSize: 10,
lineHeight: 14,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dit was

lineHeight: theme.font.extraSmall.fontSize,

oftewel 10, en die wijziging resulteert ook in de snapshot-verschillen. Was dit de bedoeling?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ik ga het maandag met Karel bespreken. Het design zegt 14 maar op dit moment is het 12 in Observation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Het design komt niet helemaal overeen met hoe wij het geïmplementeerd hebben, ofwel ik lees het anders. De juiste line height kan ofwel 12 ofwel 14 zijn, maar we houden het nu bij 12. Dat is hetzelfde als het oude InputPanel component in Observation.

Het klopt dat dit een verandering van de line height in het InputPanel in deze repo is. Gelukkig is de impact heel klein. In Observation en ObsIdentify gebruiken we dit centrale InputPanel nog niet met een label. Er is dus geen visueel verschil voor de gebruiker.

Comment thread src/theme/tokens/text.ts Outdated

export const createTextStyles = (theme: Theme) =>
({
/* eslint-disable observation/no-function-without-logging */

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We hebben in eslint.config.mjs een uitzondering voor createStyles, daar kunnen we createTextStyles aan toevoegen, dan hoef je dat niet hier te doen.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Daar zat ik al over te twijfelen. Als dat jou voorkeur heeft dan schrijf ik het nog even om.

@barry-observation
barry-observation merged commit 9d6a263 into develop Sep 14, 2026
2 checks passed
@barry-observation
barry-observation deleted the feature/new-static-text-styles branch September 14, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants