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

Tag for old tag name is not removed when changing cutom tag name #145

Open
almarouk opened this issue Dec 26, 2024 · 4 comments
Open

Tag for old tag name is not removed when changing cutom tag name #145

almarouk opened this issue Dec 26, 2024 · 4 comments

Comments

@almarouk
Copy link

When using a custom tag name, after specifying a new tag name, the new tag is added to the items that had the old tag, but the old tag is not removed.
Is this the expected behavior? Is it possible to add an option for removing the old tag in this case?

@almarouk
Copy link
Author

Another issue is that there's no message displayed if the input is not valid (after running the check):

// Notifier.showNotification('Warning', `Invalid tag string. Tag cannot contain: ${found.join(' or ')}.`, false)

and the value is not reset to the previously valid tag name (it will be reset to the default value after running manual sync).

@daeh
Copy link
Owner

daeh commented Dec 26, 2024

the old tag is not removed.
Is this the expected behavior?

That is expected behavior. Zotero makes it really easy to bulk-remove tags (by right clicking on a tag and choosing "Delete Tag..."), so I thought it'd be safer to just let the user handle cleanup rather than automating a behavior like deleting tags. Have you run into issues with that design choice or was it just unexpected?

CleanShot 2024-12-26 at 18 01 44@2x

Another issue is that there's no message displayed if the input is not valid (after running the check):
the value is not reset to the previously valid tag name (it will be reset to the default value after running manual sync).

These are both good points. I've been meaning to deal with config string validation before the preference pane is closed (rather than when the sync is run). Those updates will change the silent reversion to default into a vocal reversion to the last-valid value.

@almarouk
Copy link
Author

Have you run into issues with that design choice or was it just unexpected?

Yeah it was just unexpected and I didn't know that it was possible to remove a tag from all items in Zotero. Maybe display a popup message to notify the user after changing the name tag, possibly with a button to delete the previous tag?

These are both good points. I've been meaning to deal with config string validation before the preference pane is closed (rather than when the sync is run). Those updates will change the silent reversion to default into a vocal reversion to the last-valid value.

Yeah I think that the commonly expected behavior is for the validation check to happen right after losing focus from the input field (onchange event), followed directly by an effective change of the corresponding pref. At least that's how other settings are dealt with as far as I know.

Applying the change on close event of the preference pane doesn't feel right since the preference pane uses a dynamic logic (there's no 'save' button on its buttom).

On the other hand, an example of usage case for why it shouldn't be at close event: usually what I do is change some preferences and test the effect in Zotero's main pane while leaving the preference pane open because I'm not sure yet if that's the final pref I want (either because the pref description isn't clear so I play around with it to understand what it does, or because I'm not sure without seeing how it turns out). So it would be confusing if nothing changed in this case, especially that other prefs do work as expected here.

Btw thanks for the reply and for the plugin! I'd be happy to help with code changes as much as I can (I don't usually code with JavaScript/web based languages but I've been playing around with it for a while).

Cheers

@almarouk
Copy link
Author

I also have a few other suggestions (maybe I should put them in another issue?), for example:

  • keep a cache/index of the current state of the database for faster load in case of a big library.
  • add support for multiple root directories and their automatic sync that can be disabled/enabled individually. For example, it will be possible then to divide the databases by collections and disable sync for non-active collections to alleviate having to process a big library/database.
  • add support for addon api (currently api = {}) for further integration with other plugins. For example if there's a cache/index enabled, when adding new markdown files/notes (for example via the Actions & Tags plugin), we can call api.updateIndex(listOfItems | listOfPaths) to update the index and avoid running a full sync.
  • add support for a local REST API (possibly with the same functionalities as the internal addon api). Similar to the previous example but in case of adding files from outside of Zotero (via Obsidian for example).
  • ...

I'd be happy to discuss it with you.

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

No branches or pull requests

2 participants