Skip to content

Add embedding option in text editor#5948

Open
Borreg0 wants to merge 20 commits into
OpenSlides:mainfrom
Borreg0:5902-InternalReferences
Open

Add embedding option in text editor#5948
Borreg0 wants to merge 20 commits into
OpenSlides:mainfrom
Borreg0:5902-InternalReferences

Conversation

@Borreg0
Copy link
Copy Markdown
Contributor

@Borreg0 Borreg0 commented Mar 5, 2026

Closes #5902

@Borreg0 Borreg0 self-assigned this Mar 5, 2026
@Borreg0 Borreg0 added the feature label Mar 5, 2026
@Borreg0 Borreg0 marked this pull request as draft March 5, 2026 09:58
@Borreg0 Borreg0 marked this pull request as ready for review March 6, 2026 11:07
@Borreg0 Borreg0 assigned bastianjoel and Elblinator and unassigned Borreg0 Mar 6, 2026
@bastianjoel bastianjoel assigned Borreg0 and unassigned bastianjoel and Elblinator Mar 9, 2026
@Borreg0 Borreg0 assigned bastianjoel and Elblinator and unassigned Borreg0 Mar 9, 2026
Copy link
Copy Markdown
Member

@Elblinator Elblinator left a comment

Choose a reason for hiding this comment

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

  • 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

@Elblinator Elblinator assigned Borreg0 and unassigned bastianjoel and Elblinator Mar 9, 2026
@Borreg0 Borreg0 marked this pull request as draft March 12, 2026 09:26
@Elblinator
Copy link
Copy Markdown
Member

We talked about this issue internally and visually want something else then described above:

  • 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"

@Borreg0
Copy link
Copy Markdown
Contributor Author

Borreg0 commented Mar 17, 2026

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.

@Borreg0 Borreg0 marked this pull request as draft March 17, 2026 14:43
@Borreg0 Borreg0 force-pushed the 5902-InternalReferences branch from 8cf1663 to 67f752e Compare March 18, 2026 11:30
@Borreg0 Borreg0 marked this pull request as ready for review April 24, 2026 09:09
@Borreg0 Borreg0 assigned bastianjoel and Elblinator and unassigned Borreg0 Apr 24, 2026
@Borreg0 Borreg0 requested a review from Elblinator April 24, 2026 09:10
@bastianjoel bastianjoel assigned Borreg0 and unassigned bastianjoel and Elblinator Apr 24, 2026
Copy link
Copy Markdown
Member

@bastianjoel bastianjoel left a comment

Choose a reason for hiding this comment

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

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;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is to add a protocol if missing. Your change here makes no sense.
foo.com -> http://foo.com
http.com -> http.com

@Borreg0 Borreg0 force-pushed the 5902-InternalReferences branch from 6f0d2bb to 2b686aa Compare April 27, 2026 12:42
@Borreg0 Borreg0 requested a review from bastianjoel April 27, 2026 12:48
@Borreg0 Borreg0 assigned Elblinator and unassigned Borreg0 Apr 27, 2026
Copy link
Copy Markdown
Member

@Elblinator Elblinator left a comment

Choose a reason for hiding this comment

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

  • 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...

@Elblinator Elblinator assigned Borreg0 and unassigned bastianjoel and Elblinator May 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Embedding of motion/assignment/topic links in home/topic text

3 participants