Skip to content

Conversation

crazytonyli
Copy link
Contributor

Fixes https://linear.app/a8c/issue/CMM-783

Description

This PR introduces two improvements to the tags list UX:

  1. Selected tags are not displayed on the list.
  2. Keyboard is no longer dismissed after tapping return key to add a tag.

@crazytonyli crazytonyli added this to the 26.4 milestone Sep 24, 2025
@crazytonyli crazytonyli requested a review from kean September 24, 2025 10:16
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 24, 2025

App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number29227
VersionPR #24883
Bundle IDorg.wordpress.alpha
Commitb5ce3b8
Installation URL32kbaf11asdho
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Sep 24, 2025

App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number29227
VersionPR #24883
Bundle IDcom.jetpack.alpha
Commitb5ce3b8
Installation URL7jj53ea42glto
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@kean
Copy link
Contributor

kean commented Sep 24, 2025

This is better!

I'd try and reproduce the previous UX in the new SwiftUI-based implementation.

ScreenRecording_09-24-2025.08-03-17_1.MP4

Using it on the device, I like that:

  • The tags are displayed directly in the text field, so nothing on the screen has to move
  • It's nice that it hi lights the matched letters in auto-suggestions
  • Once you select a tag from auto-suggestions, it fills out the rest of the text in the textfield, and moves the cursor to the new empty position
  • It is faster because it searches locally

Maybe we could use an open-source implementation like https://github.com/venmo/VENTokenField or based the implementation on something like it?

@crazytonyli
Copy link
Contributor Author

The tags are displayed directly in the text field, so nothing on the screen has to move

The keyboard being dismissed after adding a new tag is a worse UX. But IMO I don't think displaying tags outside of the input field makes that much of a difference. I'm not very keen on adding a third-party library if we really need to put the tags in the text field. Nonetheless, do you think it'd be okay to keep the design as it is for now?

It is faster because it searches locally

The current implementation is based on the data view component, which searches remotely. It could be a big refactor if we switch to local search...

@kean
Copy link
Contributor

kean commented Sep 25, 2025

the keyboard being dismissed after adding a new tag is a worse UX

I'm not sure I follow what you were referring it. I agree that the keyboard should never dismiss, but the text field should be cleared when you add the text. I would also rename the placeholder to "Add tag" instead of "Search tags" – it's not just search.

do you think it'd be okay to keep the design as it is for now?

The new version also looks good. I would just make sure it reserves enough space when empty so when the first tag is added, the keyboard doesn't jump. It would be nice to add some basic animations for tag insertion too.

I'd also suggest making the text in tags 1pt larger, and making the "delete" buttons less pronounced – you'd usually have light background and dark symbol in that case. Otherwise, they become overwhelming when there are too many. It's already clear you can tap to delete.

It could be a big refactor if we switch to local search

I think it would be worth it (as a separate change?), because these requests are surprisingly slow, and it also looks strange when you have a small list of tags (<30) and the suggestions keep disappearing and appearing after a pretty noticeable delay.

I think it could be pretty straightforward if it would keep pagination for TagsListView but use non-paginated response in TagsSearchView and then it would probably be easier to dynamically switch from "user has <100 tags, so searching locally" to "user has >100 tags, so let's fire off a request every time they search".


Btw, this code in PostSettingsRow can be updated to use native NavigationLink since it's a SwiftUI screen now:

    func showTagsPicker() {
        let tagsVC = TagsViewController(blog: post.blog, selectedTags: settings.tags) { [weak self] newTagsString in
            self?.settings.tags = newTagsString
        }
        viewController?.navigationController?.pushViewController(tagsVC, animated: true)
    }

And I also noticed the following visual artifact when running it on the device:

ScreenRecording_09-25-2025.16-17-54_1.MP4

@crazytonyli
Copy link
Contributor Author

the keyboard being dismissed after adding a new tag is a worse UX

I'm not sure I follow what you were referring it.

I meant that the keyboard should not be dismissed when the user searches/adds tags by typing rather than scrolling through the list.

I'd also suggest making the text in tags 1pt larger, and making the "delete" buttons less pronounced

Thanks for the suggestions. I have updated the UI. Let me know what you think.

I think it would be worth it (as a separate change?)

I can follow this up in future PRs. I have created a linear issue: https://linear.app/a8c/issue/CMM-788

And I also noticed the following visual artifact when running it on the device:

It appears to be related to the old UIKit performance issue with the keyboard showing up for the first time. If you go to other places in the app to bring up the keyboard (like searching in the post list), the transition seems to be smoother. I have added a delay to show the keyboard in the latest commit, which should help too.

Copy link

Copy link
Contributor

@kean kean left a comment

Choose a reason for hiding this comment

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

Merging together with the other PRs with additional changes.

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.

4 participants