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

Padma t1 #202

Merged
merged 6 commits into from
Nov 17, 2023
Merged

Padma t1 #202

merged 6 commits into from
Nov 17, 2023

Conversation

daniel-belcher
Copy link
Collaborator

@daniel-belcher daniel-belcher commented Nov 17, 2023

Purpose

Round one of Padma testing concerns.

  • text changes
  • select optional sorting
  • field array append text added
  • character limits and viewable count for text areas

@daniel-belcher daniel-belcher marked this pull request as ready for review November 17, 2023 18:53
Copy link
Collaborator

@jdinh8124 jdinh8124 left a comment

Choose a reason for hiding this comment

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

Looks good from what I can see

extends React.TextareaHTMLAttributes<HTMLTextAreaElement> {}
extends React.TextareaHTMLAttributes<HTMLTextAreaElement> {
charcount?: "simple" | "limited";
charcountstyling?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this be:
charcountStyling?

Copy link
Collaborator Author

@daniel-belcher daniel-belcher Nov 17, 2023

Choose a reason for hiding this comment

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

Where it's being applied it causes a loud warning about needing to be lowercase for custom DOM attributes.

There's probably a better way to add these two properties in general.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see makes sense

Copy link
Collaborator

@benjaminpaige benjaminpaige left a comment

Choose a reason for hiding this comment

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

love it!! Thank you!

Comment on lines +89 to +98
const opts = useMemo(() => {
if (hops.sort) {
const sorted = hops.options.sort((a, b) =>
a.label.localeCompare(b.label)
);
hops.sort === "descending" && sorted.reverse();
return sorted;
}
return hops.options;
}, [hops.options, hops.sort]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

@daniel-belcher daniel-belcher merged commit 3ad3bec into webforms Nov 17, 2023
16 of 17 checks passed
@benjaminpaige benjaminpaige deleted the padma-t1 branch November 29, 2023 20:58
Copy link
Contributor

🎉 This PR is included in version 1.5.0-val.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants