Add embedding option in text editor#5948
Conversation
This reverts commit c64a1c5.
There was a problem hiding this comment.
- The new feature should still be in the dialog
- Both options should have a keyboard_arrow_down/keyboard_arrow_up icon (and the function of such)
- Both titles should have the same colour and NOT the disabled colour
- "Insert/edit link" should be changed to "Insert/edit external link"
- The option to add Home is missing right now
- The option to add more than one object is not working
- If you look at a motion with this functionality you can add a string or several motions.
- if you use this function:
- then you need to change the title to "Link a topic/motion/assignment"
- AND after you added one and then press the option again you need to add a NEW link. Right now after adding one you are visually one space behind the link, and if the option is pressed again the old link is change to the new link. Only if you are on the link (cursor directly on the last/first letter or in between) the link should be changed.
- Remove the done/clear option and if an object was selected then update the string in the selected item box directly
- The issue with the updated link if you visually are at a different place should be fixed regardless of whether or not the dialog can create one vs several links
|
We talked about this issue internally and visually want something else then described above:
|
|
About point E): The options are enabled again when another radio-button is selected. I will add a reset button again so it's more intuitive for the user. |
8cf1663 to
67f752e
Compare
bastianjoel
left a comment
There was a problem hiding this comment.
I've stumbled over too much things I don't like too quickly here. Therefore stopping my review.
Please take a look at your code again and cleanup unnecessary code, debug statements and check for obvious bugs before submitting it for review.
| this.link.href = `http://` + this.link.href; | ||
| if (this.externalUrl) { | ||
| if (!/^[a-zA-Z]+:\/\//.test(this.link.href)) { | ||
| this.link.href = this.externalUrl.includes(`http`) ? this.externalUrl : `http://` + this.externalUrl; |
There was a problem hiding this comment.
This is to add a protocol if missing. Your change here makes no sense.
foo.com -> http://foo.com
http.com -> http.com
6f0d2bb to
2b686aa
Compare
Elblinator
left a comment
There was a problem hiding this comment.
- Please also add the clear option for the Insert/edit external link
- this clear option should also just be present if "Link a topic/motion/assigment" is present (aka hide this optioon if it's not present)
- Hide the "keyboard_arrow_down" icon when only "Insert/edit external link" is present
- "Link a topic/motion/assigment" should only be present in home and the topic creating/editing
- I saw it in assignemts/participantss/moderation note. It's possible that I forgot to check other places, please make sure to only present it at home and topics
- If the "Link a topic/motion/assigment" option is opended there is a warning in the console*^1, please prevent that warning
*^1 [only part of the waring:
editor-link-dialog.component.html:111
It looks like you're using the disabled attribute with a reactive form directive. If you set disabled to true...
Closes #5902