The staff interface includes a green bar at the top of almost every page, #header_search.toptab I have been writing custom JavaScript to color this bar differently per branch. It would be much nice if you could just do this in Koha without JS.
Created attachment 167577 [details] [review] Bug 37054: DBUpdate
Created attachment 167578 [details] [review] Bug 37054: DO NOT PUSH - Schema updates
Created attachment 167579 [details] [review] Bug 37054: Add branch color to branches.tt
Created attachment 167590 [details] [review] Bug 37054: Adjust header colors if branches.branchcolor is used To test: 1. Apply patch, update database, restart services, yarn build 2. Go to Administration > Libraries 3. Edit or create a new branch. 4. You should now see a 'Branch color' option. 5. Set that to anything you want. 6. Log in as that branch. 7. Your header color should now relect that choice. 8. Log in to another branch without a set branch color. 9. The header should remain the default green.
Created attachment 167595 [details] [review] Bug 37054: DBUpdate Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167596 [details] [review] Bug 37054: DO NOT PUSH - Schema updates Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167597 [details] [review] Bug 37054: Add branch color to branches.tt Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167598 [details] [review] Bug 37054: Adjust header colors if branches.branchcolor is used To test: 1. Apply patch, update database, restart services, yarn build 2. Go to Administration > Libraries 3. Edit or create a new branch. 4. You should now see a 'Branch color' option. 5. Set that to anything you want. 6. Log in as that branch. 7. Your header color should now relect that choice. 8. Log in to another branch without a set branch color. 9. The header should remain the default green. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167609 [details] [review] Bug 37054: (follow-up) Simplify changes, add a way to clear the color field This patch cuts some of the CSS changes which I think were unnecessary and adds a way for the user to clear the color field. A text input now shows the selected hex colur value and the "Clear" button clears it. I've changed the way the patch handles the custom colors by making the color handling dependent on there being a branchcolor value. This way we don't have to alter the existing header CSS, just override it if a custom color has been chosen. To test, apply the patch and test that the color picker still works correctly and that the "Clear" button works to empty the field. Test the appearance of the search bar in a branch with a custom color and in a branch without.
Created attachment 167612 [details] [review] Bug 37054: DBUpdate Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167613 [details] [review] Bug 37054: DO NOT PUSH - Schema updates Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167614 [details] [review] Bug 37054: Add branch color to branches.tt Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167615 [details] [review] Bug 37054: Adjust header colors if branches.branchcolor is used To test: 1. Apply patch, update database, restart services, yarn build 2. Go to Administration > Libraries 3. Edit or create a new branch. 4. You should now see a 'Branch color' option. 5. Set that to anything you want. 6. Log in as that branch. 7. Your header color should now relect that choice. 8. Log in to another branch without a set branch color. 9. The header should remain the default green. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 167616 [details] [review] Bug 37054: (follow-up) Simplify changes, add a way to clear the color field This patch cuts some of the CSS changes which I think were unnecessary and adds a way for the user to clear the color field. A text input now shows the selected hex colur value and the "Clear" button clears it. I've changed the way the patch handles the custom colors by making the color handling dependent on there being a branchcolor value. This way we don't have to alter the existing header CSS, just override it if a custom color has been chosen. To test, apply the patch and test that the color picker still works correctly and that the "Clear" button works to empty the field. Test the appearance of the search bar in a branch with a custom color and in a branch without. Signed-off-by: David Nind <david@davidnind.com>
Thank you Owen! I am going to propose 2 changes: 1. I don't like that the background color of the links still changes to green when a custom color is set. In my opinion we don't need to set the background color on hover. I think that rule can be removed for the header.scss file. 2. Can we turn the filter brightness down just a bit on the '#header_search button[type="submit"]' btn? And then turn it up a bit when hovering on that element?
Created attachment 167618 [details] [review] Bug 37054: (follow-up) Remove some unneeded SCSS and ajust button brightness
Created attachment 167625 [details] [review] Bug 37054: (follow-up) Remove some unneeded SCSS and ajust button brightness Signed-off-by: Owen Leonard <oleonard@myacpl.org>
This looks great!
Please rebase!
Created attachment 171221 [details] [review] Bug 37054: DBUpdate Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171222 [details] [review] Bug 37054: DO NOT PUSH - Schema updates Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171223 [details] [review] Bug 37054: Add branch color to branches.tt Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171224 [details] [review] Bug 37054: Adjust header colors if branches.branchcolor is used To test: 1. Apply patch, update database, restart services, yarn build 2. Go to Administration > Libraries 3. Edit or create a new branch. 4. You should now see a 'Branch color' option. 5. Set that to anything you want. 6. Log in as that branch. 7. Your header color should now relect that choice. 8. Log in to another branch without a set branch color. 9. The header should remain the default green. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171225 [details] [review] Bug 37054: (follow-up) Simplify changes, add a way to clear the color field This patch cuts some of the CSS changes which I think were unnecessary and adds a way for the user to clear the color field. A text input now shows the selected hex colur value and the "Clear" button clears it. I've changed the way the patch handles the custom colors by making the color handling dependent on there being a branchcolor value. This way we don't have to alter the existing header CSS, just override it if a custom color has been chosen. To test, apply the patch and test that the color picker still works correctly and that the "Clear" button works to empty the field. Test the appearance of the search bar in a branch with a custom color and in a branch without. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 171226 [details] [review] Bug 37054: (follow-up) Remove some unneeded SCSS and ajust button brightness Signed-off-by: Owen Leonard <oleonard@myacpl.org>
rebased for current main
Created attachment 175991 [details] [review] Bug 37054: DBUpdate Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 175992 [details] [review] Bug 37054: DO NOT PUSH - Schema updates Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 175993 [details] [review] Bug 37054: Add branch color to branches.tt Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 175994 [details] [review] Bug 37054: Adjust header colors if branches.branchcolor is used To test: 1. Apply patch, update database, restart services, yarn build 2. Go to Administration > Libraries 3. Edit or create a new branch. 4. You should now see a 'Branch color' option. 5. Set that to anything you want. 6. Log in as that branch. 7. Your header color should now relect that choice. 8. Log in to another branch without a set branch color. 9. The header should remain the default green. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 175995 [details] [review] Bug 37054: (follow-up) Simplify changes, add a way to clear the color field This patch cuts some of the CSS changes which I think were unnecessary and adds a way for the user to clear the color field. A text input now shows the selected hex colur value and the "Clear" button clears it. I've changed the way the patch handles the custom colors by making the color handling dependent on there being a branchcolor value. This way we don't have to alter the existing header CSS, just override it if a custom color has been chosen. To test, apply the patch and test that the color picker still works correctly and that the "Clear" button works to empty the field. Test the appearance of the search bar in a branch with a custom color and in a branch without. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 175996 [details] [review] Bug 37054: (follow-up) Remove some unneeded SCSS and ajust button brightness Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Thank you for working on this, Lucas! I'm so excited to see this in Koha.
(In reply to Laura Escamilla from comment #33) > Thank you for working on this, Lucas! I'm so excited to see this in Koha. Hi Laura, thanks for QA! As we are just at the beginning of the cycle I'll be more strict about the guidelines for QA/SO being from people of different institutions. Sorry that this "hit you" first now :( I'd also like to get some notes on implementation. We now have opacusercss in the branches table. This is for adjusting the color in one specific spot, which could also have been achieved with a staffusercss. Is there a strong reason it was done this way? (I assume so, but always good to have it written down!) Some notes from looking at the code: 1) This uses "branch" a lot, but library should be used. https://wiki.koha-community.org/wiki/Terminology 2) I believe we are missing changes for the REST API since there are routes for library information. If this was named library_color in the db as well, we could save a mapping. 3) I think the method name should be snake case per PERL9: Subroutine naming conventions. https://wiki.koha-community.org/wiki/Coding_Guidelines
> I'd also like to get some notes on implementation. We now have opacusercss > in the branches table. This is for adjusting the color in one specific spot, > which could also have been achieved with a staffusercss. Is there a strong > reason it was done this way? (I assume so, but always good to have it > written down!) I wrote this patch to replace custom JS we have been using to show a different color header for each branch the user is logged in at. I suppose it could be done with something like `staffusercss`. But I don't see the reason for the pref, why not just use IntranetUserCSS if this is how we are going to do it? Maybe pass in a data-attribute for the logged in branch, that can be targeted with CSS. Then staff would still need to be able to write CSS, which was something I was hoping to avoid with this patch.
(In reply to Lucas Gass (lukeg) from comment #35) > > I'd also like to get some notes on implementation. We now have opacusercss > > in the branches table. This is for adjusting the color in one specific spot, > > which could also have been achieved with a staffusercss. Is there a strong > > reason it was done this way? (I assume so, but always good to have it > > written down!) > > I wrote this patch to replace custom JS we have been using to show a > different color header for each branch the user is logged in at. I suppose > it could be done with something like `staffusercss`. But I don't see the > reason for the pref, why not just use IntranetUserCSS if this is how we are > going to do it? Maybe pass in a data-attribute for the logged in branch, > that can be targeted with CSS. > > > Then staff would still need to be able to write CSS, which was something I > was hoping to avoid with this patch. Just to clarify: I didn't mean a pref, but something similar to the library specific CSS available now on the library administration page. I do see benefit in an easy to use color picker.
Created attachment 176014 [details] [review] Bug 37054: (follow-up) Rename branchcolor to library_color and use snake casing
Created attachment 176015 [details] [review] Bug 37054: DO NOT PUSH - More schema updates
Created attachment 176016 [details] [review] Bug 37054: Add API support
Created attachment 176615 [details] [review] Bug 37054: atomic update - display message only if column added
Created attachment 176616 [details] [review] Bug 37054: Replace the TT plugin method with a more generic one Let's return the whole Koha::Library object so that it can be reusable for another attribute. Keeping the CamelCase to be consistent with the rest of the other methods in this file.
1. API spec: "description: the library color" The... building? :D 2. Please provide tests 3. Color picker's default is "black". It looks like it will be black, not "default"/green. 4. IMO this should be CSS or a plugin, it's overkill to have this in Koha core. Personal opinion, not blocker. Also added 2 patches. FQA for 2, discussion for 4.
> 4. IMO this should be CSS or a plugin, it's overkill to have this in Koha > core. Personal opinion, not blocker. I've considered this as a plugin, and it would be fine IMO to go that route.