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

chore: correct Sbanken typography docs #2432

Merged
merged 7 commits into from
Jun 14, 2023
Merged

chore: correct Sbanken typography docs #2432

merged 7 commits into from
Jun 14, 2023

Conversation

eliland
Copy link
Contributor

@eliland eliland commented Jun 6, 2023 •
edited
Loading

Added documentation for Sbanken typography and fixed a few bugs and inconsistencies for the line-height docs in all themes, and involuntary markup changes caused by line breaks done by prettier.

@eliland eliland requested review from tujoworker and snorrekim June 6, 2023 09:58
@vercel
Copy link

vercel bot commented Jun 6, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
eufemia ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2023 10:55pm

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 6, 2023 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 485a38c:

Sandbox Source
eufemia-starter Configuration

@eliland eliland changed the title Docs/typography docs(Typography) Jun 6, 2023
@tujoworker tujoworker changed the title docs(Typography) chore: correct Sbanken typography docs Jun 6, 2023
tujoworker
tujoworker approved these changes Jun 6, 2023
View reviewed changes
Copy link
Member

@tujoworker tujoworker left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work 🙏 looks good!

... and as we talked about, a e2e test would be cool to get in 👍

snorrekim
snorrekim reviewed Jun 12, 2023
View reviewed changes
Copy link
Contributor

@snorrekim snorrekim left a comment

Choose a reason for hiding this comment

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

Just a small spelling mistake

packages/dnb-design-system-portal/src/docs/quickguide-designer/typography/typographic-rules.mdx Outdated Show resolved Hide resolved
@eliland
Copy link
Contributor Author

eliland commented Jun 12, 2023

Added cypress tests to catch some previously seen issues (line-heights wrong due to changed line-height-basis, p elements inside headlines, ++), and added a few others. We might want to consider adding tests for CSS that is specific for the ui theme in the future.

As a side note @tujoworker, the themeSwitch and pageNavigation tests always failed for me. Could you check if you see the same issue on your end? Suspect there might be some outdated tests there, but I didn't dig too deep.

@eliland eliland requested a review from tujoworker June 12, 2023 22:57
@tujoworker
Copy link
Member

The themeSwitch and pageNavigation tests do fail locally, because we test functionally there witch are only available in prod, such as navigation without JavaScript.

tujoworker
tujoworker approved these changes Jun 14, 2023
View reviewed changes
Copy link
Member

@tujoworker tujoworker left a comment

Choose a reason for hiding this comment

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

Nice work!

@eliland eliland merged commit c3f9661 into main Jun 14, 2023
@eliland eliland deleted the docs/typography branch June 14, 2023 12:56
tujoworker added a commit that referenced this pull request Jun 14, 2023
@tujoworker
chore(Typography): add e2e for UI
961ecd0 
Based on PR #2432
@tujoworker tujoworker mentioned this pull request Jun 14, 2023
Merged
@tujoworker
Copy link
Member

🎉 This PR is included in version 10.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snorrekim snorrekim snorrekim approved these changes

@tujoworker tujoworker tujoworker approved these changes

Assignees
No one assigned
Labels
released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eliland @tujoworker @snorrekim