I have a quick question regarding implementation of a small change in our system, and I want to hear your opinion about my little disagreement with another developer in our company.
Our working environment:
- Laravel
- AdminLTE
- Two laravel guards for 'partner' and 'staff'. Each type of user (partner/staff) has access to a different set of pages, using a different set of controllers and a different subdomain.
Admin LTE comes with some skins that you can apply to your <body>, for example 'skin-blue' theme. This is what our page looks like. Just for a comparison, if you remove the 'skin-blue' class, our website looks like this.
We were asked by our client to change the color of the top navbar for the Staff side. So, because the colors at the moment are being added by an adminLTE skin, I thought it was better to create a second theme for the staff side, calling it "skin-staff", and then in our base blade file, check for which guard is being used, and add the class accordingly.
<body class="@if(get_guard() === 'partner') skin-blue @else skin-staff @endif" ...>
I made a copy of the original skin-blue file, renamed it to skin-staff, and just changed the color of the necessary elements. I thought this was the best way to go about it, but the developer which had to review my github Pull Request said that because this was such a small change, it wasn't necessary to create a new skin. His proposed solution was to simply add the css classes in the blade file, something like:
<head>
…
<style type="text/css">
@if (get_guard() === 'staff')
.skin-blue .main-header .navbar{
background-color:#bdac3c
}
.skin-blue .main-header .navbar .sidebar-toggle:hover{
background-color:#ac9b2b
}
.skin-blue .main-header .logo{
background-color:#bdac3c;
}
… // and other classes
@endif
</style>
Now, to me this is not correct, because we are mixing the logic for staff and partner side without a clear way to differentiate them. If we use skins, we can simply say something like "The top navbar is yellow because we are using class skin-staff". And "We are using class skin-staff because we are on the Staff guard". The propositions are clear and simple. However, by adding raw CSS to our blade file, we end up with something like "The top navbar is yellow because we are using skin-blue and also we are on the Staff guard and also we have added some custom CSS for the Staff guard". The extra changes we introduce to the system don't follow the pattern used by adminLTE, to me they just look like noise. If we had to for example do this five more times, we would end up with a lot of CSS in our base blade file, which I think would look bad and would force us to eventually decide to use the skin system of adminLTE, something we could just do right away.
But, being as stubborn as I know I am, I don't know if I have the right idea or if I just want to do things my way.
What do you guys think? Is it better to create a new skin, even if most of the CSS code inside the skin file will be duplicated, but it allows us to stick to the existing way of doing things, or is it better to just add the code in the blade file and don't think more about it?
Thanks for your ideas
via Chebli Mohamed
Aucun commentaire:
Enregistrer un commentaire