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

make import listing more configurable #7308

Closed

Conversation

pmario
Copy link
Contributor

@pmario pmario commented Mar 1, 2023

This PR is an alternative approach to closed PR #7306
It does 2 things.

  • Create a new system tag: $:/tags/ImportOverwrite
  • Moves the default text out from the overWriteWarning() macro to a new tiddler: $:/core/ui/ImportListing/overwrite/default-text

So users have the ability to extend the import message area with their own info by tagging their template with new system-tag.

This attachment contains a sample configuration to play with. This info is not part of the PR.
import-listing-verbose.zip

There should be no visual difference between this PR and the v5.2.5
You need to import the JSON above to see a difference.

There is an import payload attachment, which creates a more realistic import dialogue with "state", "temp" and "blocked" tiddlers.
test-import.zip

@Jermolene if it can be merged, I'll add some docs about the new system-tag and how to use the new possibilities

@vercel
Copy link

vercel bot commented Mar 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Mar 1, 2023 at 11:29PM (UTC)

@pmario
Copy link
Contributor Author

pmario commented Mar 1, 2023

@yaisog .. You may have a look at it.

@yaisog
Copy link
Contributor

yaisog commented Mar 1, 2023

I like the configurability via the tag, as I mentioned in the other PR. This makes it modular and configurable concerning the information displayed.

Could you make it so that the original message goes into the emptyMessage of the $list instead of the the "default-text" tiddler? This way one could remove the default message without having to remove the tag from a system tiddler. I can imagine that some people find the message superfluous when they have the table. Adding the message to a configuration tiddler to get it back would be simple.
I'm also thinking of replacing the default message with a "A (newer|older) tiddler with this name already exists." if newer/older can be determined.

PS: You could also add !is[draft] to the list filter.

@pmario
Copy link
Contributor Author

pmario commented Mar 1, 2023

I'm also thinking of replacing the default message with a "A (newer|older) tiddler with this name already exists." if newer/older can be determined.

Be aware, that the default message is translatable at the moment. .. So if you can reuse translated text you should do that.

@yaisog
Copy link
Contributor

yaisog commented Mar 2, 2023

Be aware, that the default message is translatable at the moment.

Yes, I noticed. While the verbal indication as part of the message might be useful, it would add two new language strings... 🤔

@yaisog
Copy link
Contributor

yaisog commented Mar 2, 2023

I have created an alternative overwrite message for those who can live with a little more verbosity than in Mario's version:
image

There are a number of checks to only show relevant information (and hide metadata which is identical for both tiddlers), and a separate message when tiddlers seem to be identical ("seem" because I can only check the text field but not all possible fields, as they cannot be accessed by filter operators, as far as I know). Also, tiddlers with no modified date are output as such instead of blank lines.

Personally, I find it helpful to add an extra highlight to critical tiddlers, i.e. those cases where something bad is likely to happen should the user just click "Import" without thinking twice.

The definition tiddlers are here:
import-listing-yaisog.zip

I am by no means suggesting that these should become part of the core or this PR. It is simply a demonstration of how this PR could be leveraged to make the import table a little more useful (and which would make a nice plugin).

PS: In a more code-polished version, the style declarations would go into their own tiddler, of course. The way it is now they are declared anew for each conflict line of the table.

@pmario
Copy link
Contributor Author

pmario commented Mar 3, 2023

I am by no means suggesting that these should become part of the core or this PR. It is simply a demonstration of how this PR could be leveraged to make the import table a little more useful (and which would make a nice plugin).

That was the reason for this PR. It should be possible now to make more experiments, without the need to directly include more verbose info into the core. So users who want more info can use plugins.

In the past we had some user feedback that the import info is overwhelming. That's way we did reduce it to the bare minimum.

This PR should make it more flexible again. .. And it's the users choice if they want to see more info.

@pmario
Copy link
Contributor Author

pmario commented Mar 6, 2023

@Jermolene .. I would like to have this one in v5.2.6 .. Could you have a look?

@saqimtiaz
Copy link
Contributor

saqimtiaz commented Mar 6, 2023

I have created an alternative overwrite message for those who can live with a little more verbosity than in Mario's version:

It would be worth exploring an approach that allows use of the full table width as the preview mechanism does, to ensure a viable approach for smaller screen sizes.

In fact, it would be worth exploring the kinds of messages shown in the examples as previews, with perhaps the facility to automatically show the preview for certain types of tiddlers/conflicts.

@Jermolene
Copy link
Owner

Hi @pmario I'm inclined to leave this for the next release. I understand the underlying need, but I think I'd rather collect together all the outstanding import-related requests and make a plan.

@pmario
Copy link
Contributor Author

pmario commented May 6, 2023

@Jermolene Please review again.

Quote from the OP

This attachment contains a sample configuration to play with. This info is not part of the PR.
import-listing-verbose.zip

There should be no visual difference between this PR and the v5.2.5
You need to import the JSON above to see a difference.

There is an import payload attachment, which creates a more realistic import dialogue with "state", "temp" and "blocked" tiddlers.
test-import.zip

TW Default Import

image

Custom Import Initial

If import-listing-verbose.JSON is imported and the test-import.JSON is imported the import dialogue will look like this:

image

Custom Import - existing files

If all tiddlers are imported already and imported again

image

@pmario
Copy link
Contributor Author

pmario commented Jun 4, 2023

Bump in the light of: Planning for the release of v5.3.0 #7279

@pmario
Copy link
Contributor Author

pmario commented Jun 14, 2023

@Jermolene Bump for review. It does not change the existing Import dialogue. It adds the possibility for plugin authors to modify it.

Description and ZIP files needed for test testing are at: #7308 (comment)

@saqimtiaz
Copy link
Contributor

Hi @pmario, have you considered using the already extensible preview mechanism for the same purpose? For example a preview could be created that compares the metadata such as the modified date of tiddlers that would be overwritten by the import.

Not only is that an existing mechanism that could be reused, but it would also allow for the entire width of the table to be used to display that information rather than trying to fit it into that small last column.

The import mechanism is overdue for a complete rewrite in the near future and I would be hesitant to add further complexity and backwards compatibility constraints to it at this point, which might curtail our options for the rewrite.

@pmario
Copy link
Contributor Author

pmario commented Jun 14, 2023

You are right. I'll rewrite it as a plugin. So close.

@pmario pmario closed this Jun 14, 2023
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.

None yet

4 participants