-
Notifications
You must be signed in to change notification settings - Fork 8
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
Conversation
size-limit report 📦
|
Everything looks huge and unnatural with this change: https://deploy-preview-422--conversionxl-aybolit.netlify.app/?path=/story/cxl-ui-cxl-app-layout--cxl-app-layout-2-cr Original: https://conversionxl.github.io/aybolit/?path=/story/cxl-ui-cxl-app-layout--cxl-app-layout-2-cr I don't think we should edit it right now. |
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. |
@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. |
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 think we have enough important tasks to work on, no need to focus on global font change now. |
Ah i did not notice 2 links.
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. LargeSmall |
I didn't realize you can't reopen a PR after you've force pushed. Here is the new PR: #423 |
https://cxlworld.slack.com/archives/C01JABH8AHX/p1720530653218939