open-props

open-props copied to clipboard

Let me know if there's anything else I need to do. I think it might be nice to have an explanation of the benefits of using these props over an external font, but I'm not sure where it would fit. Thanks!

Run & review this pull request in StackBlitz Codeflow.

Look at all that tucked into nice little props 👍🏻
I do wonder if we need to break these modern font stacks into their own import?

wonder if we should put ^ these behind a summary details so they don't take up so much space? was cool when there were a few, but now there's a lot.

Good point about giving them more credit. I think it would also be nice to have some information explaining why people would want to use these over an external font. Maybe that explanation and better credit could be in a paragraph after the "Font Families" heading?

I've got finals this week and next week so I won't be able to do much here until after that

I've got finals this week and next week so I won't be able to do much here until after that

hope finals went well! let us know if you need help closing this out. it's a sweet addition.

They did (still waiting for some results though 😶)! Thanks! So sorry for the delay— life’s been coming at me and my ADHD has had me hyperfixating on another project (built with open-props) lol. I’ll get back to this pr soon!

I think these fonts would work best as a separate import. However, I think that the font-weights, sizes, etc should remain in the default bundle. Do you think it would be confusing to have the font-families separate from the other font props? Should the font family props be in a separate import, should all the font related props be in a separate import, or should they remain in the main bundle?

Alright, I think that's everything except if you want them split into a separate import. Do you want to keep --font-system-ui? It's from modern font stacks, but I think it's unnecessary. It is set to system-ui, sans-serif

Alright, I think that's everything except if you want them split into a separate import. Do you want to keep --font-system-ui? It's from modern font stacks, but I think it's unnecessary. It is set to system-ui, sans-serif

rad ty! i'm just returning from conferencing and ready to review this. I do think it's a good idea to include that prop as it's often missed by folks and ChromeOS for example doesn't yet support system-ui while it does sans-serif. healthy fallback habit. Thanks for asking!

I'll give this a review now 👍🏻

I think we should consider the overall approach for introducing new props in the library:

• Should we add them to the main import, as we did with Static Sizes and Drawn Borders? (While they are great, I'm still not convinced they should be in the main bundle.)
• Or should we keep everything as separate imports from now on?

I think we should consider the overall approach for introducing new props in the library:

• Should we add them to the main import, as we did with Static Sizes and Drawn Borders? (While they are great, I'm still not convinced they should be in the main bundle.)
• Or should we keep everything as separate imports from now on?

When the added props are entirely new for the current release, they should be imported separately. I fully agree with this. However, as those in this PR are touching existing stuff, it must be in the main. Otherwise, it would be breaking. And then, as proposed, fix this in the next version. I suggest proceeding with the current plan. For completeness, that is:

• map the old props to the new
• add all into the main import
• add deprecation notice in docs (as needed?)
• move everything into its self-contained import in the next version.

I've added the old values and mapped them to the new ones. Will postcss-jit-props be able to handle that correctly? Do you think a deprecation notice is needed?

The variables should be good to go. I have 2 quick questions. Do you think we need a deprecation notice for the old values? Do you think we should note that --monospace-code is different in open-props than in modern font stacks? I think it's ok to quietly remove the old font values. I also think it's ok to not add the note, but thought I'd run it by you. Thanks for all the help!

Do you think we need a deprecation notice for the old values?

I've made a new issue to hold this task, I'll try and make sure it's part of OPv2 👍🏻

Do you think we should note that --monospace-code is different in open-props than in modern font stacks?

Seems like a fair small note to add to the docs. Use a blockquote like this?

Done

No problem! Thanks for your support and time!