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

Add possibility for query parameters in link field type and ckeditor link overlay #6478

Merged
merged 5 commits into from
Jul 12, 2022

Conversation

exastion
Copy link
Contributor

@exastion exastion commented Feb 2, 2022

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related issues/PRs #6440 #6418
License MIT

What's in this PR?

This PR adds the possibility to add a query parameter to link in the editor link overlay.

Why?

In #6440 the possibility to add query parameters to Internal Links was added for the <sulu-link> tag but it was not added to the editor overlay.

@exastion exastion changed the title Enhancement/link query Add possibility for query parameters in sulu link in editor Feb 2, 2022
@exastion
Copy link
Contributor Author

exastion commented Feb 2, 2022

I managed to run the tests locally, however I do not know how to set things up to be able to preview the changes in the actual GUI.

@alexander-schranz
Copy link
Member

@exastion Thank you for the pull request. I will try to have a look at it soon as possible.

However I do not know how to set things up to be able to preview the changes in the actual GUI.

The sulu/sulu repository itself on your branch you can create an admin build npm install and npm run build then configure your database (.env.local) then build sulu admin bin/console sulu:build dev and start a webserver to log into the admin interface: php -S 127.0.0.1:8000 -t public config/router.php under http://127.0.0.1:8000/admin.

@exastion
Copy link
Contributor Author

exastion commented Feb 2, 2022

@alexander-schranz I already tried that, but it seems it was another problem, got it to work now and could test it.
I will open an issue for the problem I encountered. (#6479)

@alexander-schranz alexander-schranz added Bug Error or unexpected behavior of already existing functionality Feature New functionality not yet included in Sulu and removed Bug Error or unexpected behavior of already existing functionality labels Feb 2, 2022
@@ -57,6 +59,12 @@ export default class LinkTypeOverlay extends React.Component<LinkTypeOverlayProp
/>
</Form.Field>

{onQueryChange &&
Copy link
Contributor

Choose a reason for hiding this comment

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

would be nice to add a test case for this similar to

test('Render overlay with anchor enabled', () => {
const linkOverlay = mount(
<LinkTypeOverlay
href={undefined}
onAnchorChange={jest.fn()}
onCancel={jest.fn()}
onConfirm={jest.fn()}
onHrefChange={jest.fn()}
open={true}
options={
{
displayProperties: ['title'],
emptyText: 'No page selected',
icon: 'su-document',
listAdapter: 'column_list',
overlayTitle: 'Choose page',
resourceKey: 'pages',
}
}
/>
);
expect(linkOverlay.find('Form').render()).toMatchSnapshot();
});

Copy link
Contributor

@niklasnatter niklasnatter left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! It would be great if you could add a testcase like described above 🙂

@niklasnatter niklasnatter force-pushed the enhancement/link-query branch 2 times, most recently from 6fe614d to e29aab6 Compare July 11, 2022 14:42
@niklasnatter niklasnatter changed the title Add possibility for query parameters in sulu link in editor Add possibility for query parameters in link field type and ckeditor link overlay Jul 11, 2022
@niklasnatter niklasnatter force-pushed the enhancement/link-query branch 3 times, most recently from a640157 to 6eacbda Compare July 11, 2022 16:53
@alexander-schranz alexander-schranz merged commit aa9db2d into sulu:2.5 Jul 12, 2022
@alexander-schranz
Copy link
Member

@exastion Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in Sulu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants