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.

Sign up for GitHub

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

Jump to bottom

Parity: Port create and add from v9.4 to v 9.5 #711

Merged
merged 8 commits into from
Jan 22, 2025
Merged

Parity: Port create and add from v9.4 to v 9.5 #711

merged 8 commits into from
Jan 22, 2025

Conversation

SkeleyM
Copy link
Contributor

@SkeleyM SkeleyM commented Jan 18, 2025 •
edited
Loading

close #688

  • Ports the Create & Add tag to v9.5
  • added translation keys (only one) for English and French
  • unify use of BuildTagPanel to avoid multiple imports within functions

hope this is alright

(edit: to add images)
Clicking create opens the Add Tag modal
image

Computerdores
Computerdores suggested changes Jan 18, 2025
View reviewed changes
Copy link
Collaborator

@Computerdores Computerdores left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Mostly fine, except for two nits and two minor issues:

  • 2 Strings aren't translated
  • The search field doesn't get cleared after creating the tag (not sure this was implemented in 9.4.x but it would be more ergonomic)

tagstudio/resources/translations/en.json Outdated
@@ -197,6 +197,7 @@
"tag.color": "Color",
"tag.confirm_delete": "Are you sure you want to delete the tag \"{tag_name}\"?",
"tag.create": "Create Tag",
"tag.create_quick": "Create && Add \"{query}\"",
Copy link
Collaborator

@Computerdores Computerdores Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: I would rename this to tag.create_add to be inline with open_create_library and other keys.
(I have added the other required changes as separate comments (change2, change3))

Suggested change
"tag.create_quick": "Create && Add \"{query}\"",
"tag.create_add": "Create && Add \"{query}\"",

tagstudio/resources/translations/fr.json Outdated
@@ -115,6 +115,7 @@
"tag.add_to_search": "Ajouter à la Recherche",
"tag.aliases": "Alias",
"tag.color": "Couleur",
"tag.create_quick": "Créer && Adjouter \"{query}\""
Copy link
Collaborator

@Computerdores Computerdores Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

needed for the nit

Suggested change
"tag.create_quick": "Créer && Adjouter \"{query}\""
"tag.create_add": "Créer && Adjouter \"{query}\""

tagstudio/src/qt/modals/tag_search.py Outdated
row.setSpacing(3)

create_button = QPushButton(self)
Translations.translate_qobject(create_button, "tag.create_quick", query=query)
Copy link
Collaborator

@Computerdores Computerdores Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

needed for the nit

Suggested change
Translations.translate_qobject(create_button, "tag.create_quick", query=query)
Translations.translate_qobject(create_button, "tag.create_add", query=query)

tagstudio/src/qt/modals/tag_search.py Outdated

self.build_tag_modal: bt.BuildTagPanel = bt.BuildTagPanel(self.lib)
self.add_tag_modal: PanelModal = PanelModal(
self.build_tag_modal, "New Tag", "Add Tag", has_save=True
Copy link
Collaborator

Choose a reason for hiding this comment

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

These strings should be translated

tagstudio/src/qt/modals/tag_search.py Outdated
@@ -5,6 +5,7 @@

import math

import src.qt.modals.build_tag as bt
Copy link
Collaborator

@Computerdores Computerdores Jan 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Nit: For readability's sake I wouldn't shorten this we all have IDEs with autocompletion after all

@CyanVoxel CyanVoxel added Type: UI/UX User interface and/or user experience Priority: Medium An issue that shouldn't be be saved for last TagStudio: Tags Relating to the TagStudio tag system Type: QoL A quality of life (QoL) enhancement or suggestion labels Jan 18, 2025
@CyanVoxel CyanVoxel added this to the SQL Parity milestone Jan 18, 2025
Computerdores
Computerdores suggested changes Jan 19, 2025
View reviewed changes
Copy link
Collaborator

@Computerdores Computerdores left a comment

Choose a reason for hiding this comment

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

Everything I pointed out before seems to be fixed, however I found another small issue: Pressing return doesn't trigger "Create and Add", this patch fixes that for me:
return_fix.patch

@SkeleyM
Copy link
Contributor Author

SkeleyM commented Jan 19, 2025

I was unsure if this would be desired behaviour, thank you for implementing it for me.

CyanVoxel
CyanVoxel approved these changes Jan 22, 2025
View reviewed changes
Copy link
Member

@CyanVoxel CyanVoxel left a comment

Choose a reason for hiding this comment

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

Thank you so much for this, and thank you @Computerdores for reviewing!

@CyanVoxel CyanVoxel merged commit 441ffce into TagStudioDev:main Jan 22, 2025
5 checks passed
@SkeleyM SkeleyM deleted the port_create_and_add branch January 23, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Computerdores Computerdores Computerdores approved these changes

@CyanVoxel CyanVoxel CyanVoxel approved these changes

Assignees
No one assigned
Labels
Priority: Medium An issue that shouldn't be be saved for last TagStudio: Tags Relating to the TagStudio tag system Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience
Projects
TagStudio Development
Status: ✅ Done
Milestone
SQL Parity
Development

Successfully merging this pull request may close these issues.

[Parity]: Port "Create & Add Tag" from v9.4 to v9.5
3 participants
@SkeleyM @Computerdores @CyanVoxel