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

Changes cursor position to end of inserted image syntax, implements #2079 #2106

Merged
merged 2 commits into from Mar 6, 2018
Merged

Conversation

rsynnest
Copy link

Changes cursor position to end of text if media manager is in 'keep open' mode for adding multiple images (Issue #2079).

@mention-bot
Copy link

@rsynnest, thanks for your PR! By analyzing the history of the files in this pull request, we identified @selfthinker, @adrianheine and @akate to be potential reviewers.

Copy link
Collaborator

@phy25 phy25 left a comment

Choose a reason for hiding this comment

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

The code works well and looks good, but I have some advice in hand.


insertTags(edid, '{{' + alignleft + mediaid + opts + alignright + '|', '}}', '');
if (keepopen) {
insertTags(edid, '{{' + alignleft + mediaid + opts + alignright + '|}}', '', '');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think after the tag we can add \n\n for better composition. That is:

insertTags(edid, '{{' + alignleft + mediaid + opts + alignright + "|}}\n\n", '', '');

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would disagree. I often use the feature to insert multiple images into one article but not necessarily one after each other. More like "one here, on there and another there..." having it insert new paragraphs would be counter productive then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@splitbrain is right. I ignored this use case before.

@Klap-in Klap-in changed the title Fix for Issue #2079 Changes cursor position to end of inserted image syntax, implements #2079 Sep 1, 2017
@Klap-in
Copy link
Collaborator

Klap-in commented Sep 1, 2017

I would like that if the media manager includes just one image (or the latest if you include a couple of images), that it puts the cursor in the just included syntax (the behaviour before this change).

My proposal is to check if the cursor is already in a syntax (that is checking if }} exists after the cursor), and put the next inserted image after it.

@splitbrain
Copy link
Collaborator

@Klap-in suggestion makes much more sense to me. @rsynnest could you adjust your changes accordingly?

@rsynnest
Copy link
Author

sure thing, will push an update soon

…ady inside an image tag

If it's already inside an image tag, move the cursor to the end of the current tag.
@splitbrain splitbrain merged commit a53b370 into dokuwiki:master Mar 6, 2018
@splitbrain
Copy link
Collaborator

good work. Thanks a lot @rsynnest 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants