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

[Desktop, UI] Input, color picker improvements #633

Conversation

niikeec
Copy link
Contributor

@niikeec niikeec commented Mar 26, 2023

Changes made to Input:

  • label string - Shows label above input field
  • error string | boolean
    • boolean - Applies "error" classes to the field
    • string - Applies "error" classes to the field and shows the error below the input
  • icon Icon | ReactNode - Shows icon
  • iconPosition 'left' | 'right' default = 'left' - Determine the position of the icon
  • right ReactNode - Add extra content on the right side of the input
  • outerClassName string - Apply styles to the outer(parent) div
  • labelClassName string - Apply styles to the label
  • iconClassName string - Apply styles to the icon
  • rightClassName string - Apply styles to the right content

These changes remove reusable label code and fix input buttons clashing with the input.

image
image

Changes made to the color picker:

  • Changed react-colorful styles
  • Added hex input

image

Changes made to useDialog:

  • accepts a new closeOnSubmit: boolean prop default = true - whether to close the dialog after the form has been submitted
  • returns a new close: () => void prop - manually close the dialog

Changes made to create tag form:

  • Prevent the form from submitting a tag with an empty name

image

@niikeec niikeec requested a review from a team as a code owner March 26, 2023 22:06
@vercel
Copy link

vercel bot commented Mar 26, 2023

@niikeec is attempting to deploy a commit to the Spacedrive Team on Vercel.

A member of the Team first needs to authorize it.

@Brendonovich
Copy link
Contributor

I appreciate what you've done here, but +-500 lines of assorted changes without any prior consultation isn't something I want to review or accept. We require linking to an issue to ensure that both reviewers and contributors are on the same page about what code should be written - we don't want anyone's time and effort being wasted.

For this PR in particular, there's a number of things I would have liked to discuss beforehand (not a fan of all the className props, label + error should be on FormField) , and its scope (500 lines, 30 files) is way too large for anyone to want to review.

I'm going to close this PR for the reasons I mentioned above, but it'd be great to discuss these changes individually in issues and get them merged in separate PRs!

@niikeec
Copy link
Contributor Author

niikeec commented Mar 28, 2023

My apologies, I totally understand. I'll separate them as much as possible.

@niikeec
Copy link
Contributor Author

niikeec commented Mar 28, 2023

Opened a discussion #640

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.

None yet

2 participants