Added option to load different Admin LTE skins #6378

fastnloud · 2020-09-07T13:01:08Z

Added option to load different Admin LTE skins

Added the option skin to load different Admin LTE skins, making the body class configurable and thus allowing you to change the skin without the need to overriding the body_attributes block.

I am targeting this branch, because this option is currently missing.

Closes #6368.

Changelog

### Added
- Added option to load different Admin LTE skins

VincentLanglet · 2020-09-07T13:06:37Z

Seems nice ! You just need to fix the lint build.

I'm currently using in my project two different skins:

{% block body_attributes %}
    {% if is_granted('ROLE_ADMIN_PREMIUM') %}
        class="sonata-bc skin-yellow fixed"
    {% else %}
        class="sonata-bc skin-blue fixed"
    {% endif %}
{% endblock %}

Any idea about how we could improve the config option you created to do this ?

fastnloud · 2020-09-07T13:24:39Z

Thank you. Instead of overwriting the body_attributes you could then maybe load a different configuration file for based on the user role?

Still custom but then you do not need to overwrite the template.

VincentLanglet · 2020-09-07T13:54:05Z

Thank you. Instead of overwriting the body_attributes you could then maybe load a different configuration file for based on the user role?

How would you do this ?

wbloszyk · 2020-09-07T14:16:16Z

Thank you. Instead of overwriting the body_attributes you could then maybe load a different configuration file for based on the user role?

How would you do this ?

skins: 
    skin-black: [ priority: 1, roles: ['ROLE_USER']]
    skin-blue: null
    skin-yellow: [ priority: 10, roles: ['ROLE_PREMIUM']]

wbloszyk · 2020-09-07T14:22:50Z

src/Resources/views/standard_layout.html.twig

@@ -113,7 +115,7 @@ file that was distributed with this source code.
    </head>
    <body
            {% block body_attributes -%}
-                class="sonata-bc skin-black fixed
+                class="sonata-bc {{ _skin }} fixed


Suggested change

class="sonata-bc {{ _skin }} fixed

class="sonata-bc {% block adminlte_2_skin_class %}skin-black{% endblock %} fixed

Only this change is require to add this PR feature. We shouldn't add skin option becouse we working on add support for other templates.

@fastnloud WDYT?

I think all the css are not loaded by default, that's why the option is needed.

It is not problem. People can add css by sonata_admin.assets.extra_stylesheets.

IMO we already have the code to remove stylesheets and add new ones built in. So adding a block seems the best option to me.

Maybe some doc should be added then.

How to use another LTE skin:

Remove black stylesheet

Add you favorite stylesheet

Override the layout file

@wbloszyk that'd be another option. The thing is, that'd you'd still need to override a template block and you'd need to remove and add the template(s) manually. The option I introduced will do that automatically based on the available Admin LTE skins.

I think introducing this block is still a good idea but then I'd name it differently (e.g. sonata_body_class).

@VincentLanglet I think in your case a block like this would be very helpful so you're able to switch skins more easily based on the user role.

@fastnloud
Lets create "sonata.admin.template.skin_selector" or something simmilar. Then skin can be return by people preference(time, roles ...), can be validate by used template(AdminLTE2, AdminLTE3, custom)... Then new option like templete_skin_service can be add. skin option is not related to AdminBundle but to AdminLTE2.

Update
I mean service + interface to allow write people own service.

IMO that would bind php to css and would be bad for when we want to integrate webpack. There is a way currently in the code to replace javascript or css, and we should use that, the only need is to be able to override the main class, which is easy to do in twig with a simple block, no need to introduce complex logics

VincentLanglet · 2020-09-07T14:42:17Z

skins: 
    skin-black: [ priority: 1, roles: ['ROLE_USER']]
    skin-blue: null
    skin-yellow: [ priority: 10, roles: ['ROLE_PREMIUM']]

@wbloszyk I'm not sure to understand.
Would this work with this PR or does we need to add some code (in another PR) ?

wbloszyk · 2020-09-07T14:49:18Z

skins: 
    skin-black: [ priority: 1, roles: ['ROLE_USER']]
    skin-blue: null
    skin-yellow: [ priority: 10, roles: ['ROLE_PREMIUM']]
@wbloszyk I'm not sure to understand.
Would this work with this PR or does we need to add some code (in another PR) ?

I only explain how it can be done. IMO we shouldn't do in in option. Some service like sonata_admin.skin.selector will be better idea. The best will be #6378 (comment)

wbloszyk · 2020-09-08T08:48:58Z

👍 Add simply block
- very easy to add

👎 Add service "sonata.admin.template.skin_selector"
- complex solution
- allow return skin depends on time, roles ....

For both solution documentation about how to use skin should be add.

@sonata-project/contributors Lets vote

wbloszyk · 2020-09-08T08:52:46Z

Im for 👍.
People can add own solution for this.
Complex solution will be great. Anyway we will add new template soon and provide this service for each template will be huge additional work.

VincentLanglet · 2020-09-08T09:23:58Z

src/Resources/views/standard_layout.html.twig

@@ -113,7 +115,7 @@ file that was distributed with this source code.
    </head>
    <body
            {% block body_attributes -%}
-                class="sonata-bc skin-black fixed
+                class="sonata-bc {{ _skin }} fixed


Suggested change

class="sonata-bc {{ _skin }} fixed

class="sonata-bc {% block admin_lte_skin_class %}{{ _skin }}{% endblock %} fixed

I'm not against a skin option for simple usage, and a skin block to easily override the template when there is more complex needs (like a role dependant skin).

martinbroos · 2020-09-08T13:59:57Z

when we switch to webpack encore with admin LTE 3 things wil work different anyway because then you could override the sass variables and create your own skin. But I also think we should improve Sonata 3.x and it's usability to switch skins. An skin option like this is the most easy way to consume and would be great for the time being until we reach sonata 4.x.
So my vote would be to add the logic that is proposed in this PR but also add the twig block to allow more complex logic like @VincentLanglet showed us. By adding the block you don't have to override the complete body_attributes which contains other logic as well. Also a readme with how skin switching works helps adapting to these changes. For most people nothing changes because the default skin is black anyway.

We have settings for using select2, confirms etc. so it would be strange in my opnion to not have a skin setting and let the user remove / add stylesheets themselves.

fastnloud · 2020-09-11T11:37:18Z

I've added an additional Twig block that allows you to add more complex logic. Is this something you can work with @VincentLanglet?

src/Resources/views/standard_layout.html.twig

wbloszyk

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

git rebase -i origin/3.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
A window will show up with many lines, replace pick with fixup on every line but the first one
Close your editor, git should do its magic, and you should end up with one commit
Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

src/DependencyInjection/Configuration.php

src/DependencyInjection/SonataAdminExtension.php

tests/DependencyInjection/SonataAdminExtensionTest.php

franmomu · 2020-09-11T18:49:50Z

src/DependencyInjection/Configuration.php

@@ -134,6 +134,26 @@ public function getConfigTreeBuilder()
                        ->booleanNode('sort_admins')->defaultFalse()->info('Auto order groups and admins by label or id')->end()
                        ->booleanNode('confirm_exit')->defaultTrue()->end()
                        ->booleanNode('js_debug')->defaultFalse()->end()
+                        ->scalarNode('skin')


What about using enumNode, so there is no need of manual validation.

VincentLanglet · 2020-09-18T12:11:26Z

Thanks @fastnloud

fastnloud force-pushed the skin branch from 6848788 to 6a4e73f Compare September 7, 2020 13:18

wbloszyk closed this Sep 7, 2020

wbloszyk reviewed Sep 7, 2020

View reviewed changes

wbloszyk reopened this Sep 7, 2020

VincentLanglet requested a review from a team September 7, 2020 14:49

VincentLanglet reviewed Sep 8, 2020

View reviewed changes

fastnloud force-pushed the skin branch from 6a4e73f to 470d479 Compare September 11, 2020 11:33

VincentLanglet reviewed Sep 11, 2020

View reviewed changes

src/Resources/views/standard_layout.html.twig Outdated Show resolved Hide resolved

wbloszyk previously approved these changes Sep 11, 2020

View reviewed changes

phansys requested changes Sep 11, 2020

View reviewed changes

src/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

src/DependencyInjection/SonataAdminExtension.php Outdated Show resolved Hide resolved

tests/DependencyInjection/SonataAdminExtensionTest.php Outdated Show resolved Hide resolved

VincentLanglet previously approved these changes Sep 11, 2020

View reviewed changes

franmomu reviewed Sep 11, 2020

View reviewed changes

fastnloud dismissed stale reviews from VincentLanglet and wbloszyk via bfa1c3a September 14, 2020 14:58

fastnloud force-pushed the skin branch from 8beaa16 to bfa1c3a Compare September 14, 2020 14:58

Added option to load different Admin LTE skins

914993c

fastnloud force-pushed the skin branch from bfa1c3a to 914993c Compare September 14, 2020 15:06

wbloszyk approved these changes Sep 18, 2020

View reviewed changes

wbloszyk requested review from franmomu and phansys September 18, 2020 08:32

wbloszyk requested review from jordisala1991, wbloszyk, VincentLanglet and a team September 18, 2020 08:33

wbloszyk approved these changes Sep 18, 2020

View reviewed changes

phansys approved these changes Sep 18, 2020

View reviewed changes

VincentLanglet approved these changes Sep 18, 2020

View reviewed changes

VincentLanglet merged commit e51dafe into sonata-project:3.x Sep 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added option to load different Admin LTE skins #6378

Added option to load different Admin LTE skins #6378

fastnloud commented Sep 7, 2020

VincentLanglet commented Sep 7, 2020

fastnloud commented Sep 7, 2020

VincentLanglet commented Sep 7, 2020

wbloszyk commented Sep 7, 2020

wbloszyk Sep 7, 2020 •

edited

VincentLanglet Sep 7, 2020

wbloszyk Sep 7, 2020

jordisala1991 Sep 7, 2020

VincentLanglet Sep 7, 2020

fastnloud Sep 8, 2020

wbloszyk Sep 8, 2020 •

edited

jordisala1991 Sep 8, 2020

VincentLanglet commented Sep 7, 2020

wbloszyk commented Sep 7, 2020

wbloszyk commented Sep 8, 2020 •

edited

wbloszyk commented Sep 8, 2020

VincentLanglet Sep 8, 2020

martinbroos commented Sep 8, 2020

fastnloud commented Sep 11, 2020

wbloszyk left a comment

franmomu Sep 11, 2020

VincentLanglet commented Sep 18, 2020

	class="sonata-bc {{ _skin }} fixed
	class="sonata-bc {% block adminlte_2_skin_class %}skin-black{% endblock %} fixed

	class="sonata-bc {{ _skin }} fixed
	class="sonata-bc {% block admin_lte_skin_class %}{{ _skin }}{% endblock %} fixed

Added option to load different Admin LTE skins #6378

Added option to load different Admin LTE skins #6378

Conversation

fastnloud commented Sep 7, 2020