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

Add min and max properties to number field #306

Merged
merged 4 commits into from
Dec 21, 2023

Conversation

jperasmus
Copy link
Contributor

tl;dr;

Tiny PR to address #270

details

I wasn't sure if it started to make sense to pull the NumberField into its own component or add the 2 new props to the existing DefaultField. Went for the pragmatic approach. Happy to split it out as well if needed.

Copy link

vercel bot commented Dec 20, 2023

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

Name Status Preview Comments Updated (UTC)
puck-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 21, 2023 0:20am
puck-docs ✅ Ready (Inspect) Visit Preview Dec 21, 2023 0:20am

Copy link

vercel bot commented Dec 20, 2023

@jperasmus is attempting to deploy a commit to the Measured Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

This is great, thanks @jperasmus!

If you have time, would you mind adding it to the demo app for the Columns and Flex components?

  • Here for Columns, min 0, max 12
  • Here and here for Flex, min 0, no max

If not, don't worry. Once this is merged, I'll add it to the docs.


```tsx {5,6,10} copy showLineNumbers
const config = {
categories: {
typography: {
components: ["HeadingBlock", "ParagraphBlock"],
label: "Text",
title: "Text",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added this extra small fix to the docs that I noticed was wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Ah nice one ty

@jperasmus
Copy link
Contributor Author

Made the changes as requested and also updated the docs with a basic example showing the 2 new optional fields.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Works beautifully! Thanks for updating the docs and fixing my typos, too!

@chrisvxd chrisvxd merged commit 4932a6e into measuredco:main Dec 21, 2023
4 checks passed
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.

2 participants