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.

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

feat(cxl-lumo-styles): increase default font size #422

Closed
wants to merge 1 commit into from

Conversation

anoblet
Copy link
Collaborator

@anoblet anoblet commented Jul 9, 2024

@anoblet anoblet requested review from lkraav and pawelkmpt July 9, 2024 14:41
Copy link

github-actions bot commented Jul 9, 2024

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 44.22 KB (+0.04% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.89 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 29 KB (+0.03% 🔺)
packages/cxl-ui/pkg/dist-web/vendor.js 138.23 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-institute.js, 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 275.84 KB (+0.02% 🔺)

@pawelkmpt
Copy link

@pawelkmpt pawelkmpt closed this Jul 10, 2024
@lkraav
Copy link

lkraav commented Jul 10, 2024

Everything looks huge and unnatural

Overuse of hyperboles? Definitely worth a look @heshfekry. Yes, after resetting the base, maybe some individual elements might need adjustment, but overall everything flows into better visibility state quite well.

@heshfekry
Copy link

@lkraav Impossible to say definitively without comparison images.

Can we organize that somehow @pawelkmpt ?

I am net getting a this is completely off the cards vibe though. More of some things just need a tweak.

@pawelkmpt
Copy link

@lkraav Impossible to say definitively without comparison images.

Can we organize that somehow @pawelkmpt ?

I made screenshots but they didn't reflect the difference. Therefore I pasted these links. If you open them in two different tabs you'll spot the difference. It's clearly visible.

I am net getting a this is completely off the cards vibe though. More of some things just need a tweak.

I think we have enough important tasks to work on, no need to focus on global font change now.

@heshfekry
Copy link

Ah i did not notice 2 links.

I think we have enough important tasks to work on, no need to focus on global font change now.

Discussing on an improvement isn't always a drop everything and do it call to action. This type of task lives in the low hanging fruit task

We are deciding now whether it is worth pursuing or not. We can prioritise later.


Based on what i see i think the biggest upgrade is still long form p text. Increasing only the p font could be enough for now.

Everything else needs to much work and testing if we are to treat this as a quick upgrade. Things like space in menu items and action bar etc. Things could break across the board if this is a true global change.

Large

image

Small

image

@anoblet
Copy link
Collaborator Author

anoblet commented Jul 11, 2024

I didn't realize you can't reopen a PR after you've force pushed. Here is the new PR: #423

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.

4 participants