📝 Typography documentation #2247

martalalik · 2022-05-25T08:57:54Z

Resolves #452

oddvernes · 2022-08-15T13:46:25Z

packages/eds-core-react/src/components/Dialog/DialogHeader.tsx

@@ -8,7 +8,7 @@ const StyledDialogHeader = styled.div(({ theme }) => {
  return css`
    display: flex;
    justify-content: space-betweene;


This one is also typoed 🙃

oddvernes · 2022-08-15T13:57:55Z

packages/eds-core-react/src/components/Typography/Typography.docs.mdx

+</ul>
+
+Please do not use <code>light</code> in digital interfaces except in special cases where the font size is over <code>48px</code>.
+


Perhaps a note about how the equinor font is not included in eds by default and has to be added (and how)

mimarz · 2022-08-16T07:35:19Z

packages/eds-core-react/src/components/Typography/Typography.stories.tsx

-  grid-gap: 32px;
-  justify-items: start;
+const SBCard = styled(Card)`
+  border: 1px solid ${mossgreen100};


Suggest removing the border and maybe using the elevation prop on Card instead?

mimarz · 2022-08-16T07:36:04Z

packages/eds-core-react/src/components/Typography/Typography.stories.tsx

+const SBCard = styled(Card)`
+  border: 1px solid ${mossgreen100};
+  max-width: 290px;
+  display: table-cell;


Why do you need to set table-cell for Card?

trash, is done

mimarz · 2022-08-16T07:37:40Z

packages/eds-core-react/src/components/Typography/Typography.stories.tsx

-    storyDescription:
-      'Variants ordered by their required group parameter. Example `<Typography group="input" variant="label"></Typography>`. Variants under `heading` and `paragraph` don’t require the group parameter and are therefore not included here.',
+export const GroupsOverview = () => {
+  return Object.entries(typography).map(([key, group]) => (


This over shows the private property _modes (denoted by the _ in name). Suggest removing this using .filter() before .map()

mimarz · 2022-08-16T07:39:13Z

packages/eds-core-react/src/components/Typography/Typography.stories.tsx

-      Custom token
-    </Typography>
-  </Grid>
+export const Custom: Story<TypographyProps> = () => (


Maybe change this story to be about the token property? Its not really custom as its just examples of different combinations using props.

mimarz · 2022-08-16T07:40:08Z

packages/eds-core-react/src/components/Typography/Typography.tsx

  link?: boolean
+  /** Typography colors. */
  color?: ColorVariants | string
  token?: Partial<TypographyType>


Missing description

mimarz · 2022-08-16T07:41:51Z

packages/eds-core-react/src/components/Dialog/DialogHeader.tsx

@@ -8,7 +8,7 @@ const StyledDialogHeader = styled.div(({ theme }) => {
  return css`
    display: flex;
    justify-content: space-betweene;
-    algin-items: center;
+    align-items: center;


Uhm, suggest making a separate issue for tracebility for this as the above line is also wrong "space-betweene"....

Wasn’t Stylelint extension behind this idea?

Probably, I am getting the align-items as error in my IDE but not the justify-content on, which has the wrong value.

Thats right, justify-content haven't been underline.

done, new issue #2417

Suggest reverting these changes and do the fixes in a separate PR for traceability

mimarz · 2022-08-18T07:27:36Z

packages/eds-core-react/src/components/Typography/Typography.docs.mdx

+<ul>
+  <li><code>color</code>,</li>
+  <li><code>fontFamily</code>,</li>
+  <li><code>fontSize</code>,</li>
+  <li><code>fontWeight</code>,</li>
+  <li><code>fontStyle</code>,</li>
+  <li><code>fontFeature</code>,</li>
+  <li><code>lineHeight</code>,</li>
+  <li><code>textAlign</code>,</li>
+  <li><code>textTransform</code>,</li>
+  <li><code>textDecoration</code>,</li>
+  <li><code>letterSpacing</code>.</li>
+</ul>


Good suggestion! Maybe make this example closer to how it would look in code
For example:

const myToken = { color, fontFamily, fontSize, fontWeight, lineHeight, textAlign, textTransform, textDecoration, letterSpacing, }

mimarz · 2022-08-18T07:28:26Z

packages/eds-core-react/src/components/Dialog/DialogHeader.tsx

@@ -8,7 +8,7 @@ const StyledDialogHeader = styled.div(({ theme }) => {
  return css`
    display: flex;
    justify-content: space-betweene;
-    algin-items: center;
+    align-items: center;


Suggest reverting these changes and do the fixes in a separate PR for traceability

mimarz

A suggestion on change of wording to better describe token property, else it looks good 👍

packages/eds-core-react/src/components/Typography/Typography.docs.mdx

…ocs.mdx Co-authored-by: Michael Marszalek <[email protected]>

martalalik changed the title ~~📝 Typography documentation~~ 🛑 Typography documentation May 31, 2022

mimarz mentioned this pull request Jun 9, 2022

Create documentation for Links #432

Closed

4 tasks

martalalik changed the title ~~🛑 Typography documentation~~ 🚧 Typography documentation Aug 10, 2022

martalalik force-pushed the docs/ML-452-typography branch 3 times, most recently from b91d4ff to c74b725 Compare August 15, 2022 06:42

martalalik marked this pull request as ready for review August 15, 2022 13:41

martalalik requested review from mimarz, vnys and oddvernes as code owners August 15, 2022 13:41

martalalik changed the title ~~🚧 Typography documentation~~ 📝 Typography documentation Aug 15, 2022

oddvernes reviewed Aug 15, 2022

View reviewed changes

mimarz suggested changes Aug 16, 2022

View reviewed changes

martalalik force-pushed the docs/ML-452-typography branch from 35dcb4a to cad0fab Compare August 16, 2022 11:42

martalalik requested review from mimarz and oddvernes August 16, 2022 11:43

mimarz suggested changes Aug 18, 2022

View reviewed changes

martalalik changed the title ~~📝 Typography documentation~~ 🚧 Typography documentation Aug 18, 2022

martalalik added 8 commits August 18, 2022 11:05

📝 Add docs file

5d921dd

📝 Add Introduction

69ef0f8

🎨 Update Introduction

1435a9b

📝 Update argTab description

8c355a2

🎨 Add Usage

8a703c0

🎨 Add Examples

49e8fc0

📝 Add Overview

75dbc0f

✏️ fix docs

a467d4b

martalalik force-pushed the docs/ML-452-typography branch from cad0fab to a467d4b Compare August 18, 2022 09:05

martalalik added 2 commits August 18, 2022 14:01

📝 Update example

9aedb6f

🔥 Reverse typo fix in Dialog

9e0617d

martalalik changed the title ~~🚧 Typography documentation~~ 📝 Typography documentation Aug 18, 2022

martalalik requested a review from mimarz August 18, 2022 12:04

mimarz approved these changes Aug 18, 2022

View reviewed changes

packages/eds-core-react/src/components/Typography/Typography.docs.mdx Outdated Show resolved Hide resolved

Update packages/eds-core-react/src/components/Typography/Typography.d…

a76a24a

…ocs.mdx Co-authored-by: Michael Marszalek <[email protected]>

martalalik merged commit 2b85a88 into develop Aug 19, 2022

martalalik deleted the docs/ML-452-typography branch August 19, 2022 06:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

📝 Typography documentation #2247

📝 Typography documentation #2247

martalalik commented May 25, 2022

oddvernes Aug 15, 2022

martalalik Aug 16, 2022

oddvernes Aug 15, 2022

martalalik Aug 15, 2022 •

edited

Loading

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 16, 2022

martalalik Aug 16, 2022

martalalik Aug 16, 2022

mimarz Aug 18, 2022

mimarz Aug 18, 2022

martalalik Aug 18, 2022

mimarz Aug 18, 2022

mimarz left a comment

		</ul>

		Please do not use <code>light</code> in digital interfaces except in special cases where the font size is over <code>48px</code>.

📝 Typography documentation #2247

📝 Typography documentation #2247

Conversation

martalalik commented May 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martalalik Aug 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mimarz left a comment

Choose a reason for hiding this comment

martalalik Aug 15, 2022 •

edited

Loading