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

Limit Number of Tags #548

Merged
merged 4 commits into from
Apr 22, 2024
Merged

Limit Number of Tags #548

merged 4 commits into from
Apr 22, 2024

Conversation

Jackson-Williams-15
Copy link
Contributor

This only allows 6 tags to be selected, @epadams suggested we do 3 character tags only, which i think is fine. Alternatively we could keep the amount of characters the same as we have it now, but limit the amount to tags to 3.

@epadams
Copy link
Contributor

epadams commented Apr 21, 2024

Need more opinions before we limit the characters to a certain length, 3 might be too short. Things like "casual" or "strategy" are longer but probably would be commonly used. I do think we should set an overall limit though, not sure if this has been done in the API yet. We don't want 50 character length tags. I think the best options is to either limit the number of tags to 3/4, or expand the card width. @jbytes1027 @evan-scales @AaronKeys what do you think?

@jbytes1027
Copy link
Contributor

I think 4 tags =<10 characters each would work find and would probably fit.

@AaronKeys
Copy link
Contributor

I think the 10-character limit is a good idea. Having 4 tags seems like a strange number to have. I think it should be 3 or 5, but if y'all want 4, that's fine with me.

@epadams
Copy link
Contributor

epadams commented Apr 22, 2024

Sounds good to me, I'll add that we should probably add the error props so that it looks consistent and gives an indication to the user what is wrong. Also will need to change the text included from #554 to match the new limit

@Jackson-Williams-15
Copy link
Contributor Author

I set the character limit to 10 and the overall limit to 4. I also added the error helpertexts.

tag.limit.mp4

@epadams @jbytes1027 @AaronKeys

@epadams
Copy link
Contributor

epadams commented Apr 22, 2024

looks really good, ill test it in a couple of minutes

Copy link
Contributor

@epadams epadams left a comment

Choose a reason for hiding this comment

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

lgtm

@epadams epadams marked this pull request as draft April 22, 2024 20:53
@epadams epadams marked this pull request as ready for review April 22, 2024 20:53
@Jackson-Williams-15 Jackson-Williams-15 merged commit e23d530 into main Apr 22, 2024
2 checks passed
@epadams epadams deleted the LimitTags branch April 22, 2024 20:57
@epadams epadams mentioned this pull request Apr 23, 2024
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.

No limit on tags and said tags will eventually not show up on post if too many are selected
4 participants