Skip to content

change: [M3-9408] - Remove min length validation for tag - #11944

Merged
hasyed-akamai merged 4 commits into
linode:developfrom
hasyed-akamai:M3-9408-remove-min-validation-for-tag
Apr 9, 2025
Merged

hasyed-akamai merged 4 commits into
linode:developfrom
hasyed-akamai:M3-9408-remove-min-validation-for-tag

Conversation

@hasyed-akamai

@hasyed-akamai hasyed-akamai commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

Description 📝

This PR removes the validation requiring a minimum of 3 characters when adding tags. Additionally, it adds validation to handle the edge case where the tag is an empty string or contains only spaces (e.g., " ").

Changes 🔄

  • Removed the 3-character minimum validation for adding tags.
  • Added validation to show an error when the tag is an empty string or contains only spaces (e.g., " ").

Target release date 🗓️

Preview 📷

Before After
BEFORE.mov
AFTER.mov
BEFORE.mov
AFTER.mov

How to test 🧪

Reproduction steps

  • Navigate to all locations where tags can be added.
  • Attempt to add a tag with fewer than 3 characters or a tag that is just spaces (e.g., " ").
  • Verify that no validation error occurs for tags with fewer than 3 characters and that an error is shown for tags with only spaces.

Verification steps

  • Ensure that tags with fewer than 3 characters can be added without issues.
  • Verify that tags with only spaces or empty strings trigger a validation error.
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@hasyed-akamai hasyed-akamai self-assigned this Apr 1, 2025
@hasyed-akamai
hasyed-akamai marked this pull request as ready for review April 1, 2025 11:29
@hasyed-akamai
hasyed-akamai requested a review from a team as a code owner April 1, 2025 11:29
@hasyed-akamai
hasyed-akamai requested review from carrillo-erik and harsh-akamai and removed request for a team April 1, 2025 11:29
@hasyed-akamai
hasyed-akamai force-pushed the M3-9408-remove-min-validation-for-tag branch 2 times, most recently from 164ad00 to 67b5156 Compare April 2, 2025 06:57

@carrillo-erik carrillo-erik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I verified the changes as described in PR description, did not see any issues. My suggestion would be to add some unit tests to the TagsInput component.

The e2e test failure should be resolved once the PR for that is merged.

@hasyed-akamai
hasyed-akamai force-pushed the M3-9408-remove-min-validation-for-tag branch 3 times, most recently from 84f76ea to f9a65c1 Compare April 7, 2025 10:09
@hasyed-akamai

Copy link
Copy Markdown
Contributor Author

Thanks @carrillo-erik for suggesting the unit test for the TagsInput component. I've added it—feel free to review!

@harsh-akamai harsh-akamai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good ✅

@hasyed-akamai hasyed-akamai added the Add'tl Approval Needed Waiting on another approval! label Apr 8, 2025

@carrillo-erik carrillo-erik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the suggested tests. I'm approving now just need to resolve merge conflicts.

@github-project-automation github-project-automation Bot moved this from Review to Approved in Cloud Manager Apr 8, 2025
@hasyed-akamai hasyed-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Apr 9, 2025
@hasyed-akamai
hasyed-akamai force-pushed the M3-9408-remove-min-validation-for-tag branch from f9a65c1 to da87fa8 Compare April 9, 2025 05:48
@linode-gh-bot

Copy link
Copy Markdown

Cloud Manager UI test results

🎉 540 passing tests on test run #11 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing540 Passing4 Skipped117m 42s

@hasyed-akamai
hasyed-akamai merged commit 15285da into linode:develop Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge!

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants