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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend editors API #23224

Merged
merged 2 commits into from Dec 6, 2018
Merged

Extend editors API #23224

merged 2 commits into from Dec 6, 2018

Conversation

@dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Dec 3, 2018

Pull Request for Issue #17665 .

Summary of Changes

This PR introduces one more method to our Joomla.Editors API:

getSelection() { /* return the current selection */ }

Why?
Well it's a missing piece of the puzzle but mainly because it allows DEVs to do more advanced stuff instead of always replacing some selected text.

As this is a PR for the issue linked above, the menu button is the only place where this API is used at the moment. Also this part (in media/com_menus/js/admin-items-modal.js might be removed depending on the decision about B/C break, check the notes bellow)

Testing Instructions

Apply patch select some text in the editor and then through the menu button (xtd_button) try to insert a link! The link will have as text the selected text in the editor instead of the default menu name.

Repeat the process for all three core editors: none, tinyMCE and codemirror

Expected result

When you link to menu items etc, if you highlight text and link it the selected text gets replaced.

Actual result

When you link to menu items etc, if you highlight text and link it the selected text becomes the link text.

Preview

screenshot 2018-12-03 at 20 19 12
screenshot 2018-12-03 at 20 19 26

Documentation Changes Required

This PR is not breaking any API but it changes the behaviour. Eg

  • old behaviour: either some text is selected or nothing then the link will have the default menu text and the selected text gets replaced
  • new behaviour: if some text is selected then that will become the text of the menu item. If nothing is selected the link will have the default menu text.

@mbabker @wilsonge please make up your mind on the B/C here

@tonypartridge this one is for you 馃槈

@tonypartridge
Copy link
Contributor

@tonypartridge tonypartridge commented Dec 3, 2018

@wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Dec 5, 2018

I'm happy with this new proposed behaviour. Is there a reason we don't have an implementation for codemirror?

@dgrammatiko
Copy link
Contributor Author

@dgrammatiko dgrammatiko commented Dec 5, 2018

Is there a reason we don't have an implementation for codemirror

Yes! Basically the Joomla's API for the editors is a one to one match for the some public functions of code mirror (naming methods is hard 馃槈). BTW the images above showcase that this is working with codemirror

@wilsonge
Copy link
Contributor

@wilsonge wilsonge commented Dec 5, 2018

Cool. One test here and I'm happy

@infograf768
Copy link
Member

@infograf768 infograf768 commented Dec 6, 2018

I have tested this item successfully on 939cfa6

Tested fine here also in multilingual


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23224.

@wilsonge wilsonge merged commit 12094d7 into joomla:staging Dec 6, 2018
3 of 4 checks passed
3 of 4 checks passed
@joomla-cms-bot
JTracker/HumanTestResults Human Test Results: 1 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wilsonge wilsonge added this to the Joomla 3.9.2 milestone Dec 6, 2018
@dgrammatiko dgrammatiko deleted the dgrammatiko:Joomla-API-editor branch Dec 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants