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 sorting for snippet area / default snippets by title instead of key #7204

Merged
merged 4 commits into from
May 2, 2024

Conversation

stollr
Copy link
Contributor

@stollr stollr commented Nov 17, 2023

Q A
Bug fix? no
New feature? no
BC breaks? in a way, but only very low break
Deprecations? no
Fixed tickets -
Related issues/PRs -
License MIT
Documentation PR -

What's in this PR?

Changed the sorting of the snippet areas. They are sorted by title instead of their key (which is not visible in the frontend).

Why?

Currently the snippet areas are sorted by their key. But this makes it harder to scan the list for the user because titles may have the wrong order.

Note

I am using PHP's Collate class for comparing the string. But this is only available if the intl extension is loaded. So I have added a fallback to simple strcmp function, because the intl extension is not a requirement by Sulu.
But I am unsure how to handle this in the unit tests. Is it okay how it is done?

@alexander-schranz
Copy link
Member

I like the idea. What I think we need to test here if the template uses translation-keys if that is correctly handled already.

As Sulu will always support 2 ways of define a title:

Have every meta inlined this way:

    <meta>
        <title lang="en">Example</title>
        <title lang="de">Beispiel</title>
    </meta>

or using a translation key:

    <meta>
        <title>app.example</title>
    </meta>

I think the second one sadly is not everywhere correctly target in the snippet area. See #7188. Maybe @mamazu can have a look if this would break his pull request or if we fine to go with it.

@mamazu
Copy link
Contributor

mamazu commented Apr 29, 2024

Well, in the current version of Sulu using translations for the snippet areas is not supported. However if you merge my PR this PR would still work as the sorting happens in the controller and the translations are resolved in a compiler pass. So feel free to merge this.

@alexander-schranz alexander-schranz added the UX Affecting the end user label May 2, 2024
stollr and others added 2 commits May 2, 2024 12:31
Currently the snippet areas are sorted by their key. But this makes it harder to scan the list for the user because titles may have the wrong order.
@alexander-schranz alexander-schranz changed the title Sort snippet areas in admin frontend by title instead of key Add sorting for snippet area / default snippets by title instead of key May 2, 2024
@alexander-schranz alexander-schranz merged commit 5835f82 into sulu:2.6 May 2, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX Affecting the end user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants