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 anchor-field to MediaLinkTypeOverlay #7231

Merged
merged 6 commits into from
May 2, 2024

Conversation

spackmat
Copy link
Contributor

@spackmat spackmat commented Dec 13, 2023

Q A
Bug fix? no
New feature? sort of
BC breaks? no
Deprecations? no
Fixed tickets fixes #7082
Related issues/PRs #7082
License MIT
Documentation PR sulu/sulu-docs#

What's in this PR?

This PR adds an anchor field to the MediaLinkTypeOverlay (as a copy from the internal link overlay, that already has one).

Why?

As discussed in #7082 at least for PDF-media an URL fragment can be very useful (to jump to a specific page with #page=12).

Example Usage

Links to a media files (mainly PDF files) can now get an anchor like page=12 within the overlay form (just like links to pages) that is appended to the link's URL like ...longdocument.pdf#page=12.

I didn't find any documentation for the link overlays, so there is no documentation PR,

To Do

Nothing from my point of view. Update: Added a test to cover that field.

@brabli
Copy link

brabli commented Mar 20, 2024

Would be great to see this merged!

@alexander-schranz alexander-schranz added the Feature New functionality not yet included in Sulu label Apr 29, 2024
As discussed in #7082 at least for PDF-media an URL fragment can be very useful (to jump to a specific page with `#page=12`). This PR adds that field (as a copy from the internal link overlay).
Just a little test for the new anchor field.
to match linting rules
to match linting rules.
Copy link
Contributor Author

@spackmat spackmat left a comment

Choose a reason for hiding this comment

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

Fixed the linting errors.

to match linting rules.
Copy link
Contributor Author

@spackmat spackmat left a comment

Choose a reason for hiding this comment

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

Added trailing comma to match more linting rules.

@alexander-schranz
Copy link
Member

Thx, I will update the snapshot tests, mostly a little bit tricky if your system may run on none english system.

@alexander-schranz alexander-schranz merged commit 2358057 into sulu:2.6 May 2, 2024
6 of 8 checks passed
@alexander-schranz
Copy link
Member

@spackmat Thank you!

@spackmat spackmat deleted the patch-1 branch May 2, 2024 12:56
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.

None yet

3 participants