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

Multilang permalinks for Writer field #6668

Open
wants to merge 4 commits into
base: develop-minor
Choose a base branch
from

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Sep 12, 2024

Description

It looks to me like PhpUnit again ignores a test due to the method name, ::url does have unit tests.

Summary of changes

  • ::permalink() now returns a localized permalink in a multilang setup
  • Link dialog inserts localized permalinks when a current language is given

Reasoning

This should resolve #5551.
When language.detect setting is disabled, Kirby has no way to automatically figure out to which language to route a normal permalink. This is why the permalinks need to include the language code themselves, representing a permanent link to that page in this specific language. #6312 added the routing support for this. But this PR adds that those language-based permalinks are exposed in the right places:

  • when put out via $page->permalink()
  • when inserting a permalink in the writer field HTML

Additional context

I hope I have covered all cases to now convert not just normal permalinks but language-based permalinks back and forth with UUIDs and normal IDs in the Panel code.

@afbora It would be super valuable to get your expert testing skills thrown at this to be sure.

Changelog

Fixes

  • Multilang: Writer field now inserts the correct permalink for the current content language
  • Multilang: $page->permalink() returns a language-based permalink for the current content language

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@distantnative distantnative added the type: bug 🐛 Is a bug; fixes a bug label Sep 12, 2024
@distantnative distantnative self-assigned this Sep 12, 2024
@distantnative distantnative linked an issue Sep 12, 2024 that may be closed by this pull request
@distantnative distantnative marked this pull request as ready for review September 12, 2024 19:40
@distantnative distantnative added this to the 4.5.0 milestone Sep 12, 2024
@distantnative distantnative requested review from a team September 12, 2024 19:41
@afbora
Copy link
Member

afbora commented Sep 13, 2024

Do you think we need language prefix for file permalinks? Is there any difference both /@/file/tWNX7ZrzJUvS9Vad and /en/@/file/tWNX7ZrzJUvS9Vad? AFAIK files are singular objects. But page contents are different according to access uri.

@distantnative
Copy link
Member Author

@afbora Valid point. I'll check what's the best way to do just pages.

@distantnative distantnative force-pushed the fix/5551-multilang-permalinks branch 2 times, most recently from 3865e8b to cf04c08 Compare September 14, 2024 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Is a bug; fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[V4] UUID permalinks always redirect to default language page
2 participants