made action wrap text more flexible + added color red feature

[alecmaly]

Made action wrap text more flexible. You can now specify stringPrefix and stringPostfix independently as opposed to just a single wrapString property.

Added a color text red feature as an example.

P.S. I planned to keep this in my own fork, however, this may be useful enough to roll into your master branch. Feel free to reject this pull request if you feel it does not fit the needs of the plugin's userbase.

[tessus]

Oh, and please do not commit any changes to package-lock.json

[tessus]

I have been thinking about this for a while.

Why red? As an example this might not cut it because most people probably won't build their own extension. Maybe we should allow people to specify 2 or 3 pairs of pre- and postfixes in settings. We don't have to take care of shortcuts, because they can then be set via the shortcut editor.

What do you think?

[alecmaly]

I was just solving my own issue after adding color in about 20 places in my notes and saw someone else on the forum asking for it as well.

I LOVE your idea for making it more dynamic!!

Sorry about that package-lock.json commit. I'll have that removed.

A couple questions:

1. The package.json file. I incremented the version number, do you think I should revert this change also?
2. As for 2-3 custom prefix/postfix settings. Do you think we should default the first one to coloring the text a certain color, just as an example? I'm also thinking we would want to hide the icons on the taskbar (is that the proper name?). Or even, what FontAwesome icons would we even choose? A few details to think about in this implementation, however, I 100% agree with you that it is the right approach.

I'd have to take a closer look at how to update the settings panel and don't have much time right now, so perhaps next week I'll be able to revisit this.

[tessus]

Sorry about that package-lock.json commit.

No worries. It's not always clear when to commit lock files.

incremented the version number, do you think I should revert this change also?

Nope, it's fine, we can just keep it at 1.2.0 until we release the new version. Theoretically it would make sense to increase the version to x.y.z-dev after a release, but the thing is one might not know which one to bump (patch or minor). So we can leave it at 1.2.0 or if there will be many more commits after this PR we could also change it to 1.2.0-dev.

Do you think we should default the first one to coloring the text a certain color

For sure. That was my intention anyway. With regard to the others we can just pick ones that might be most needed, or ask on the forum by either asking an open question or do a poll. I think I added a topic for this plugin when I released it. We can use that one to discuss with the community....

[alecmaly]

@tessus,

So I was playing around and this is what I came up with. https://github.com/alecmaly/joplin-plugin-menu-shortcut-toolbar/commit/236bfe0012e8ec5fe6ced3e0c897378b42607f03

Functionally, it works.

In terms of code, I feel it's a bit dirty/clunky, however, I'm struggling to think of a way to clean it up. I tried testing the setting SettingItemType.Array and SettingItemType.Object thinking there could be an array of prefix/postfix pairs, but the setting wouldn't display on the settings page.

The settings page looks like this: Not as clean as I would have hoped. More work to be done here.

Hotkey settings page:

I'd love to hear if you can think of a cleaner way to do this since you are more familiar with the design of the Joplin plugin system. Just wanted to get some feedback before diving deeper into this solution.

[tessus]

The code looks ok, except 2 minor issues. You have quoted the constant, which is why it's not using the constant but the name of the constant. We might want to use a loop for the custom prefix stuff. Let me think about that.

From the screenshot I suggest 2 things:

• we should make clear somehow that only if pre/postfix are both set, the custom wrap will be active (maybe mentioning this in the documentation is enough)
• for the remove toolbar icon settings we might not need the pre/postfix in the text. I think it's more than enough to just use the label, otherwise it might look confusing.

[alecmaly]

So I did some more experimenting and have a second proposal. I tried to avoid this the first time because it kind of breaks some of the clean design; that said, it may be worth it.

This update will allow a dynamic number of custom wrap fields: https://github.com/alecmaly/joplin-plugin-menu-shortcut-toolbar/commit/6f7cd6684327dfb07199e1dc857ab14eb0c3b28e

A few notes:

• in settings.ts I execute await joplin.settings.registerSettings(PLUGIN_SETTINGS); twice. It seems I need to register the dynamic number of custom wrap fields setting first in order to pull the current value out of the database as register the dynamic number of prefix/postfix field settings? I don't know if this is kosher, but it works. Looking for your guidance on if this is acceptable practice.
• The custom wrap fields logic is split. Your original code is clean. There is a common.ts file that defines the constants that are used in both the index.ts and settings.ts file. However, with a dynamic number of custom wrap fields, I could not think of a way to maintain this structure. I default the first custom wrap field's prefix/suffex here in settings.ts and it's accelerator here in index.ts. Obviously, this is not ideal - so if you have any suggestions to clean this up I'd be happy to hear them.
• I have removed the ability to hide these wrap field icons from the menu bar for the sake of simplicity in this POC. I can work on adding them back if you think it's worth the effort.
• I have set the maximum number of custom wrap fields to 9. Once the number 10 is reached, it makes the Hotkey settings page sort order. We can increase this to 99 and make numbers 1-9 display as 01-09 to maintain sort order, however, I'm thinking 9 custom fields is probably ok. What do you think?

For context, the settings page looks like this:

1. The number of dynamic wrap text prefix/postfix pairs (requires restart)
2. Setting prefix/postfix pairs (does not require restart)
3. BUG: Notice how the Postfix of the last field was not displayed in the settings page. This happens sometimes. I've only seen it when loading as a development plugin:

Do you have any idea why this happens? I find it strange that it didn't register the Postfix setting, however it did register the prefix, which would have been in the same loop. https://github.com/alecmaly/joplin-plugin-menu-shortcut-toolbar/commit/6f7cd6684327dfb07199e1dc857ab14eb0c3b28e#diff-bb57a3bd912abc3ec2e729cb8a743838487677a5517683d0c8913a3619ac296aR91

Clearly it didn't fail to register the Prefix setting and halt code execution. There is also no error in the debug console. I'm not sure what's going on here. Reloading Joplin fixes the issue. Have you seen similar behavior with plugins loaded in development mode, or do you see an error in my code that may cause this behavior?

Summary

Anyway, just curious to hear what direction you would prefer the plugin to go moving forward. A static 2/3 fields with prefix/postfix or a dynamic number of them + a strange bug to fix (not sure how I would resolve that bug, it seems inconsistent but I've seen it a couple times).

[tessus]

I will have a look at it in the next few days. I am currently busy with some stuff.

But I think the dynamic option is the way to go. When it comes to the toolbar icons, I have to look into maybe generating ones. Maybe something with the letters C W and n. (Or allow setting them via a non-public setting.) I also would like to have the option to remove them, because not all people like toolbar icons. But all these things can be done at a later point. We don't have to put everything into one release and/or commit.

You have done a great job so far.

It seems I need to register the dynamic number of custom wrap fields setting first in order to pull the current value out of the database as register the dynamic number of prefix/postfix field settings?

Maybe start a discussion on the forum in the category https://discourse.joplinapp.org/c/development/plugins/19 I am sure other plugin devs experienced similar issues before.

I can work on adding them back if you think it's worth the effort.

We can do that later. As mentioned before, we don't have to do all at once.

I'm thinking 9 custom fields is probably ok. What do you think?

I agree.

BUG: Notice how the Postfix of the last field was not displayed in the settings page.

Also something to start a topic on the forum in the Development > Plugins category. I haven't seen this problem before, but maybe somebody else has.

[tessus]

@alecmaly Hello. Happy New Year!

Did you see my previous comment? I think starting a discussion on the forum is the way to go.

Btw, can you create a 2nd PR or change this one to include https://github.com/alecmaly/joplin-plugin-menu-shortcut-toolbar/commit/6f7cd6684327dfb07199e1dc857ab14eb0c3b28e ?

[tessus]

@alecmaly are you still around?

[alecmaly]

Hello,

I've become very busy and this totally fell off my radar. I may be able to get back to it within the next few weeks. I may get a couple hours within the next week to evaluate another PR and posting on the forum. I want to look over the code a bit to see if I can make it any cleaner.

My apologies for not communicating or being able to give a more concrete timeline on my availability.

I hope you're having a nice start to your 2022!

[tessus]

No worries. All good. I just wanted to know if you are still interested in working on this. I don't need a timeline. I leave this PR open for now and we'll pick up where we left off.

Thanks, my start into 2022 was ok. I hope the same is true for you as well.

[tessus]

This has been stale for a very long time. I'll close it for now. If you are still interested in working on, we can open it again.