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

[v5] View buttons with link should accept Query language similiar to page preview #6861

Closed
tobimori opened this issue Dec 10, 2024 · 5 comments · Fixed by #6901
Closed

[v5] View buttons with link should accept Query language similiar to page preview #6861

tobimori opened this issue Dec 10, 2024 · 5 comments · Fixed by #6901
Assignees

Comments

@tobimori
Copy link
Contributor

tobimori commented Dec 10, 2024

Description

CleanShot 2024-12-10 at 13 16 16@2x

This doesn't work - yet Query language works in options.preview.

buttons:
  preview:
    icon: youtube
    text: socialstar.youtube.video
    theme: red
    link: "https://youtu.be/{{ page.uuid.id }}"

Same for the text, so it works with translations.

Your setup

Kirby Version v5-beta.1

@medienbaecker
Copy link
Contributor

I've noticed the view buttons do not accept a translation array either, is this the same issue?

buttons:
  email:
    text:
      de: Kontakt
      en: Contact
    icon: email
    link: mailto:[email protected]
    theme: info

This throws Kirby\Panel\Ui\Buttons\ViewButton::__construct(): Argument #16 ($text) must be of type ?string, array given.

Also, looking at the other stuff in my blueprint I'm wondering why this is text instead of label. I personally find it confusing/inconsistent.

@distantnative
Copy link
Member

@medienbaecker it correlates to the text prop of k-button https://lab.getkirby.com/public/lab/docs/k-button

@distantnative
Copy link
Member

@tobimori @medienbaecker I have a first draft at #6901 - do you think query/i18n support is needed for more props than listed currently?

@distantnative distantnative linked a pull request Jan 11, 2025 that will close this issue
6 tasks
@medienbaecker
Copy link
Contributor

Is there a reason for limiting the props with translation/query support? I could imagine having a theme (positive, negative, ...) or icon (warning, check mark, ...) depending on a query would be very useful. Any prop probably 😅

@distantnative
Copy link
Member

distantnative commented Jan 11, 2025

Performance (minimal) and just keeping the implementation clean and easy to maintain. Of course we could throw it at everything, but in the long run this isn't so helpful but rather might lead us into dead ends. That's why I think it does make sense to make intentional choices. E.g. I don't think there is much value in adding i18n support for theme or icon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants