Skip to content

Conversation

@zumoshi
Copy link
Contributor

@zumoshi zumoshi commented Jun 2, 2025

You mentioned in #72 that create tag page should not allow creating tags with spaces in them.

While fixing that I also noticed that the dialogue lets you actually make an empty tag!

And that the ui does not in any way react to receiving an error (e.g. duplicate tag).

So this PR tries to fix all of the above. CreateTag mutation now checks for empty tag and tags with spaces, and the ui shows the error similar to how other ui elements (e.g. RelativeDateTimeSelector) do by adding a red text under the input box.

@zumoshi
Copy link
Contributor Author

zumoshi commented Jun 3, 2025

Added tests (also edited your tests since createTag's tests had spaces in them...). Also noticed that updateTag has the same issue while adding tests so added a check there + tests for that.

And changed the error reporting to snackbar for both addTag and editTag (which also ignored the error).

Also rebased the PR on master.

let me know if there are any other changes needed.


if newKey != nil && *newKey != key {
if strings.Contains(*newKey, " ") {
return nil, fmt.Errorf("tag must not contain spaces")
Copy link
Member

Choose a reason for hiding this comment

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

Here the transaction must be closed otherwise it will block further requests. (already did the change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, missed that one.

Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

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

Thanks.

@jmattheis jmattheis merged commit 52c28e6 into traggo:master Jun 4, 2025
1 check passed
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.

2 participants