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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NodeBundle] Show url slug in wysiwyg #1938

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

waaghals
Copy link
Contributor

Q A
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets #1910

This PR adds back the slug for the selected page in wysiwyg editors. This allows end users to recognize what page they have picked. The id is still used internally, so even when the slug changes, this correct link is generated in the frontend. (Although the slug in the wysiwyg stays the same)

@Devolicious Devolicious added this to the 5.2.0 milestone Aug 28, 2018
@Devolicious Devolicious changed the base branch from 5.0 to master August 28, 2018 14:53
Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @waaghals, your PR needs some changes

  • your PR title should look like [SomeBundle] Fixed some code

@Devolicious Devolicious removed this from the 5.2.0 milestone Feb 7, 2019
@acrobat
Copy link
Member

acrobat commented Jun 25, 2020

@waaghals can you rebase this pr so the conflicts are solved and can you provide some screenshots to see how this will look in the cms? Thanks!

Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR needs some changes

  • This PR seems to need a milestone of a minor release.

@ProfessorKuma ProfessorKuma added this to the 5.6.0 milestone Jun 25, 2020
@acrobat acrobat modified the milestones: 5.6.0, 5.7.0 Jun 25, 2020
@dannyvw
Copy link
Contributor

dannyvw commented Sep 18, 2020

@waaghals ping ;)

@waaghals
Copy link
Contributor Author

Old

Old

New

New

Note: Different installations, so id is different between screenshots.

With the new link format, the editor can see to which page the link is referring to.

@waaghals waaghals changed the title Show url slug in wysiwyg [NodeBundle] Show url slug in wysiwyg Sep 21, 2020
Copy link

@ProfessorKuma ProfessorKuma left a comment

Choose a reason for hiding this comment

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

Hi @, your PR passed all our requirements.

Thank you for contributing!

@waaghals
Copy link
Contributor Author

@acrobat rebased the PR and I've added a screenshot with the old and the new link format.

@acrobat
Copy link
Member

acrobat commented Sep 21, 2020

@waaghals Thanks for the update! Can you check the failing travis build and styleci?
I'm thinking if it would be possible to make the wysiwyg always display the correct slug in the textarea? With some callback function on load of the editor or ...?

Do you have an idea to "fix" this in ckeditor @dbeerten?

@dannyvw
Copy link
Contributor

dannyvw commented Sep 21, 2020

@acrobat Done

@acrobat acrobat modified the milestones: 5.7.0, 5.8.0 Oct 12, 2020
@acrobat acrobat modified the milestones: 5.8.0, 5.9.0 Mar 17, 2021
@acrobat acrobat modified the milestones: 5.9.0, 5.10.0 Oct 10, 2021
@acrobat
Copy link
Member

acrobat commented Oct 12, 2021

This still needs some work on the editor side as it would be more userfriendly to always show the correct slug for a link. I'm removing the milestone so we finish this somewhere in 6.x

@acrobat acrobat removed this from the 5.10.0 milestone Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants