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

feat(cxl-lumo-styles): update typography #381

Merged
merged 3 commits into from
Jan 9, 2024
Merged

feat(cxl-lumo-styles): update typography #381

merged 3 commits into from
Jan 9, 2024

Conversation

anoblet
Copy link
Collaborator

@anoblet anoblet commented Jan 3, 2024 •
edited
Loading

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 3, 2024 •
edited
Loading

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 71.14 KB (+2.07% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.89 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 28.84 KB (+0.02% 🔺)
packages/cxl-ui/pkg/dist-web/vendor.js 140.5 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js, packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js, packages/cxl-ui/pkg/dist-web/cxl-ui.js, packages/cxl-ui/pkg/dist-web/manifest.js, packages/cxl-ui/pkg/dist-web/unresolved.js, packages/cxl-ui/pkg/dist-web/vendor.js 253.52 KB (+0.58% 🔺)

@anoblet anoblet force-pushed the anoblet/feat/typography branch from 11c48f0 to 3b7a575 Compare January 3, 2024 12:57
@heshfekry
Copy link

heshfekry commented Jan 3, 2024 •
edited
Loading

  • Font weights are wrong and not to the spec from Clickup card - Please ensure font sizes and weights are as follows

H1 - Raleway font, weight 900, size 65px, line height 65px
H2 - Raleway font, weight 900,
H3 - Raleway font, weight 700,
H4 - Raleway font, weight 500,

  • Change line height on all h1 and 2 to var(--cxl-lumo-line-height-xxs)

  • Change the dashboard headers to the following styles so they sync with global header styles. Now they are totally seperated. (Another style sync fail after repeat attempts - cc @pawelkmpt )

Frame 5509 (2)

  • Course names here should be h3 style

image

Ill QA again after these changes @anoblet as its hard to judge the rest without some of these changes.

@heshfekry
Copy link

Task linked: CU-86ayy0gy1 Change fonts

@anoblet anoblet marked this pull request as draft January 4, 2024 12:22
@anoblet anoblet force-pushed the anoblet/feat/typography branch 4 times, most recently from 8864c04 to 35861ba Compare January 4, 2024 16:12
@anoblet anoblet marked this pull request as ready for review January 4, 2024 16:13
@anoblet anoblet force-pushed the anoblet/feat/typography branch from 35861ba to 06c864f Compare January 4, 2024 16:46
@pawelkmpt
Copy link

pawelkmpt commented Jan 4, 2024 •
edited
Loading

Screenshot 2024-01-04 at 21 34 18

cxl-course-card needs header adjustment for .name too (screenshot from WPS)

@anoblet anoblet force-pushed the anoblet/feat/typography branch from ebd24bd to b7af527 Compare January 5, 2024 05:11
@anoblet anoblet requested a review from pawelkmpt January 5, 2024 14:57
@pawelkmpt
Copy link

Storybook looks different than compiled aybolit applied on WPS. Mostly due the primary font family. On WPS all text uses Lato. Storybook still has Roboto and Libre on some elements eg. menu.

@heshfekry do we want everything Lato?

Screenshot 2024-01-08 at 11 00 33

Dashboard cards need tweaks for headings. Titles have too big margins.

localhost_8080_dashboard_

@heshfekry
Copy link

@pawelkmpt

Yes we do want everything lato. We shouldn't see roboto or liber anywhere. Keep things simple as our typography system, is sub optimal.

Agree that the cards need a tweak.

@pawelkmpt
Copy link

I fixed margins cc @anoblet

Screenshot 2024-01-08 at 13 11 29
Screenshot 2024-01-08 at 13 11 43

and found one more thing, certificates new vs old style.
Screenshot 2024-01-08 at 13 05 59

@heshfekry
Copy link

This title can be changed to Raleway font-weight 900 for sure

image

Rest can stay the same

@pawelkmpt pawelkmpt force-pushed the anoblet/feat/typography branch from c2301ea to 1fe67a4 Compare January 8, 2024 15:03
@pawelkmpt
Copy link

I cleaned up commits and squashed them into just 3

@pawelkmpt pawelkmpt force-pushed the anoblet/feat/typography branch 2 times, most recently from afadfd8 to 5de154c Compare January 8, 2024 15:18
lkraav
lkraav reviewed Jan 8, 2024
View reviewed changes
packages/storybook/.storybook/preview-head.html
@@ -3,4 +3,4 @@
// @see https://bugs.chromium.org/p/chromium/issues/detail?id=946975
delete Document.prototype.adoptedStyleSheets;
</script>
<link rel="stylesheet" id="cxl-font-css" href="https://fonts.googleapis.com/css?family=Roboto:300,300i,400,400i,700,700i,900|Libre+Baskerville:400,400i,700" type="text/css" media="all">
<link rel="stylesheet" id="cxl-font-css" href="https://fonts.googleapis.com/css?family=Roboto:300,300i,400,400i,700,700i,900&family=Libre+Baskerville:400,400i,700&family=Lato&family=Raleway:wght@400;500;700;900" type="text/css" media="all">
Copy link

Choose a reason for hiding this comment

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

Same, why not kill off deprecated font family loading, or?

@pawelkmpt pawelkmpt force-pushed the anoblet/feat/typography branch from 5de154c to ff250fd Compare January 9, 2024 12:05
@pawelkmpt pawelkmpt merged commit 34b2e8c into master Jan 9, 2024
5 checks passed
@pawelkmpt pawelkmpt deleted the anoblet/feat/typography branch January 9, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lkraav lkraav lkraav requested changes

@heshfekry heshfekry Awaiting requested review from heshfekry

@pawelkmpt pawelkmpt Awaiting requested review from pawelkmpt

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anoblet @heshfekry @pawelkmpt @lkraav