-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
Refactor link dialog into ES module #2796
Refactor link dialog into ES module #2796
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2796 +/- ##
==========================================
+ Coverage 95.90% 95.92% +0.01%
==========================================
Files 225 225
Lines 6103 6129 +26
==========================================
+ Hits 5853 5879 +26
Misses 250 250 ☔ View full report in Codecov by Sentry. |
9315da2
to
dbfe1a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work. some minor naming nits :)
fac431e
to
601bab5
Compare
601bab5
to
75a21e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
b794403
to
d964b98
Compare
4edaf72
to
955708d
Compare
81ad9d5
to
875ba7f
Compare
Use our default page select web component also in the link dialog. This simplifies the link dialog (a bit) and removes an Handlebar template, which isn't necessary anymore.
Remove the Javascript code which selected the tab in link dialog and moved it into the view components. Also prefilled the forms to remove these part as well from the Coffeescript.
Moved the dom id - logic into a separate web component. This reduce the amount of necessary Javascript in link dialog and moves that into a isolated web component.
The link dialog was handling the link button and the TinyMCE implementation. Now the dialog is working with a promise and will respond to the caller with the link configuration. The calling method can make the decision to insert the link into her environment.
Migrate the Link Dialog from CoffeeScript to Javascript. This will also trim the internal class structure a bit to be better readable.
remove the usage of jQuery in LinkDialog and replace it with vanilla Javascript. Also restructure smaller parts of the class to improve the overview.
875ba7f
to
0fba444
Compare
0fba444
to
4f734be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works like charm now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app/assets/javascripts/tinymce/plugins/alchemy_link/plugin.min.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue on the anchor tag already exists in current main
and is not related to this change.
#createLink(linkOptions) { | ||
const invalidInput = | ||
linkOptions.type === "external" && | ||
!linkOptions.url.match(Alchemy.link_url_regexp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should validate earlier in the submit form event
It was previously a dom id - select and technically it is search for dom nodes with ids. It also adds an enabled and disabled state on the dom id - select. Also separate both dom id selects into slightly different components and add an enable and disable mechanic to alchemy-select.
653cf26
to
9db3ab8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 very last (promise 🙏🏻) change that would simplify this even more.
This is amazing work and I cannot wait to merge this 😁
9db3ab8
to
97b3056
Compare
Remove unnecessary data-link-target - attribute. It is not necessary anymore and can be removed. Also the support for different kinds of targets was added like _top, _self, etc. Also remove a lot of unnecessary transformation and store target with the default HTML syntax.
97b3056
to
4dc39a2
Compare
What is this pull request for?
Checklist