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

Tags Normalization #828

Merged
merged 8 commits into from
Jul 30, 2020
Merged

Tags Normalization #828

merged 8 commits into from
Jul 30, 2020

Conversation

jleandroperez
Copy link
Contributor

@jleandroperez jleandroperez commented Jul 29, 2020

Details:

In this PR we're bringing over several enhancements introduced in this macOS Issue

  • Encapsulates Tag Hash generation in a NSString extension
  • Encapsulates Tag Name validation in a (new) NSString extension
  • Updates Tag Lookup mechanism, so that comparison is based on Hash rather than on Strings. Details here!

@aerych Sir!! since you've validated the first part, mind taking a quick peek? (nothing big should have changed!!)
Thank you!!

/cc @dmsnell NOW we're talkin'! 😃

Ref. #824

Scenario: Note Editor

  1. Checkout commit cd4f251
  2. Press over the top right button to add a new Note
  3. Press over the Tags Editor
  • Verify that new tags can have at most 5 characters (this is for testing! actual limit is 256 char).
  • Verify that special characters are allowed (parentheses, comma).
  • Verify that pressing a space "commits" the text you've typed.

IMPORTANT: Length is enforced against encoded tag name. Using non alphanumeric characters will yield a much lower limit!

Scenario: Tags Editor

  1. Checkout commit cd4f251
  2. Press over the top left button to reveal the Tags Editor
  3. Long press over any tag
  • Verify that pressing Delete effectively removes the new Tag.
  • Verify that pressing Rename allows you to edit the Tag Name (up to 5 characters, for testing purposes!).
  • Verify that during a rename OP pressing spacebar "Commits" the edit.

IMPORTANT: Length is enforced against encoded tag name. Using non alphanumeric characters will yield a much lower limit!

Release

These changes do not require release notes.

@peril-automattic
Copy link

peril-automattic bot commented Jul 29, 2020

You can trigger an installable build for these changes by visiting CircleCI here.

@jleandroperez jleandroperez self-assigned this Jul 29, 2020
@jleandroperez jleandroperez added this to the 4.23 milestone Jul 29, 2020
@jleandroperez jleandroperez added the enhancement Improve existing functionality. label Jul 29, 2020
@jleandroperez jleandroperez requested a review from aerych July 29, 2020 22:01
@jleandroperez jleandroperez marked this pull request as ready for review July 29, 2020 22:05
Copy link
Member

@aerych aerych left a comment

Choose a reason for hiding this comment

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

Looks pretty good @jleandroperez :) I spotted one tiny thing and left a mention about it in the code. Other than that I think this is good to go.

Verify that new tags can have at most 5 characters

I was only able to add four characters so maybe an off by 1 issue somewhere?

Simplenote/Classes/NSString+Simplenote.swift Outdated Show resolved Hide resolved
@jleandroperez
Copy link
Contributor Author

Thank you sir!!!

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

Successfully merging this pull request may close these issues.

2 participants