django-admin-interface
django-admin-interface copied to clipboard
fabiocaccamo
feat: Allow changing foreground/background colors
name: Pull request about: Submit a pull request for this project assignees: fabiocaccamo
Describe your changes I added 4 customization options to Themes :
- Body background color
- Foreground color
- Quiet color
- Loud color
And extended French translation for the newly added fields. I couldn't find places where loud color was used, but it felt awkward to add the option for the quiet color without the loud color.
Related issue #291
Checklist before requesting a review
- [x] I have performed a self-review of my code.
- [NA] I have added tests for the proposed changes.
- [x] I have run the tests and there are no errors.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
e895a05) 97.35% compared to head (d03f7e9) 97.44%.
Additional details and impacted files
@@ Coverage Diff @@
## main #352 +/- ##
==========================================
+ Coverage 97.35% 97.44% +0.08%
==========================================
Files 38 40 +2
Lines 416 430 +14
==========================================
+ Hits 405 419 +14
Misses 11 11
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 97.44% <100.00%> (+0.08%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
Not sure why the linting fails, it seems to be related to the French translation I added, but the error isn't very clear about what's wrong...
EDIT: I compiled the new .po to .mo with msgfmt django.po -o django.mo and added it to PR
The linting keeps failing, maybe I should drop the translation commit?
It's my first time updating locale, maybe I'm doing this wrong @fabiocaccamo ?
@BMourguesFieldbox have you followed these steps? https://github.com/fabiocaccamo/django-admin-interface#translate-into-another-language
@BMourguesFieldbox have you followed these steps? https://github.com/fabiocaccamo/django-admin-interface#translate-into-another-language
Sorry I missed it, I've gone through the proper commands and repushed ! :)
@fabiocaccamo Just a reminder if you have some time to pull the trigger on the CI :)
@Shriukan33 sorry for the delay in doing the review, but I'm quite busy these days and I'm not able to find the time. I will do it as soon as possible, this has highest-priority in my open-source todo list.
@Shriukan33 sorry for the delay in doing the review, but I'm quite busy these days and I'm not able to find the time. I will do it as soon as possible, this has highest-priority in my open-source todo list.
No problem, I understand ! Life gets in the way sometimes, good luck in whatever keeps you busy :)
@fabiocaccamo Is there something blocking this from being merged? Would be awesome if we could get this in there. This project has everything I'm looking for except this feature.
@selected-pixel-jameson I haven't had time to review and test this PR yet.
@fabiocaccamo Is there something blocking this from being merged? Would be awesome if we could get this in there. This project has everything I'm looking for except this feature.
In the meantime, you could use my fork as source in your requirements like so :
django==5.*
git+https://github.com/Shriukan33/django-admin-interface.git@Allow-changing-background-color
Note that you need to have git installed on the image if you're using docker.
@BMourguesFieldbox I'll take a look, but I'm kind of having concerns even moving forward with this solution as it sounds like the maintainer is potentially running into time constraint issues, which is understandable, I just don't want to implement a solution that in the end is going to run into maintenance issues.
@BMourguesFieldbox @Shriukan33 @selected-pixel-jameson I'll try to review this as soon as possible!
@BMourguesFieldbox I'll take a look, but I'm kind of having concerns even moving forward with this solution as it sounds like the maintainer is potentially running into time constraint issues, which is understandable, I just don't want to implement a solution that in the end is going to run into maintenance issues.
Understandable, this lib doesn't do much aside from offering a way to edit the admin templates, so it doesn't really relate to app's features, I'd say it's quite low risk (unless you have lots of features in the admin)
@BMourguesFieldbox @Shriukan33 @selected-pixel-jameson I'll try to review this as soon as possible!
Thanks ! :D
Hey, just a heads up I rebased on current main, as it had like 50 new commits behind :)