-
Notifications
You must be signed in to change notification settings - Fork 185
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
Initialize link dropdown description #894
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR! This would be a much nicer UX for source mode but it would result in a different UX for WYSIWYG and source mode. I'm not sure if that's a good idea or not. It's not really possible to show the selection in the description when in WYSIWYG mode as it could contain an image or something else that can't be represented in text. Could possibly show a placeholder like |
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.
Look good! Just missed one of the calls to _dropDown
.
@@ -563,13 +563,14 @@ var defaultCmds = { | |||
|
|||
// START_COMMAND: Link | |||
link: { | |||
_dropDown: function (editor, caller, cb) { | |||
_dropDown: function (editor, caller, selected, cb) { |
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.
This causes an error in WYSIWYG mode as the call to _dropDown
in exec:
hasn't been updated to pass the selected
parameter so cb
is is being treated as selected
and there is no cb
passed.
Thank you for the review! I'm sorry I did not catch that one issue - I will fix it. I totally understand if you don't like the asymmetry with wysiwig mode. Something Awful is using SCEditor for the buttons primarily and I'm gonna keep thinking about how to make those work better, but I will do my best to test with wysiwig mode anything that I push upstream. I could see removing the description field in wysiwig mode exclusively, but, in my opinion, it is very important in source mode -it simultaneously simplifies the process of making a adding a link to a post, and also teaches new users the syntax for doing that directly. A version where selecting stuff in wysiwig mode produced a placeholder would be neat for sure but also a much larger project than simply populating a text input with plaintext. I would like it if we also used wysiwig mode some day but our set of accepted bbcode is large and weird and we'd basically end up writing a full bbcode parser in javascript. We have lower hanging fruit than that. |
At present, when you select text and use the "link" command, a dropdown menu appears for the url itself and the description. However, the initial selected text is not propagated to the "description" box, which is confusing. Populate this box with any text that was selected before the url button was hit.