[#1675] Added extra_css field to SiteConfiguration and master template #740
Conversation
Bartvaderkin
force-pushed
the
feature/1675-css-override-admin
branch
from
9b38dfe
to
503eb8a
Compare
| grid-column-gap: 2em; | ||
| &__column { | ||
| font-family: monospace; | ||
| font-size: 95%; |
There was a problem hiding this comment.
What's the reason for this 5% decrease? I can barely tell the difference from looking at the screen.
There was a problem hiding this comment.
To make it slightly smaller because monospace is so chonky.
src/open_inwoner/utils/itertools.py
Outdated
| """ | ||
| split `iterable `in parts of `chunk_size` length | ||
|
|
||
| implements Python 3.12 itertools.batched() |
There was a problem hiding this comment.
-
Cool, but is there a reason for not simply copying over the function definition from Python 3.12?
-
If we do type annotations, they should be consistent I think. So either
def batched(iterable: Iterable, chunk_size: int)ordef batched(iterable, n), letting thenspeak for itself (see the docs). -
Perhaps a different name for the module to avoid confusion with the inbuilt
itertools?
There was a problem hiding this comment.
- This seems good enough. We should update python in general but that is out of scope.
- I updated the type hint, with generics
- I renamed the module to
iteration
|
|
||
| def test_clean_stylesheet_allows_any_string(self): | ||
| """ | ||
| proofs we need to escape this if we print in html |
There was a problem hiding this comment.
Do you mean "proves"?
It's good to know that we need to do this, hence it should be documented. But what's the point of proving it via a test? I would prefer to have this information included in the docstring of utils.clean_stylesheet, where it's more prominent.
There was a problem hiding this comment.
Yes.
The problem with comments about code behaviour is they go stale.
… renamed & clarified some things
|
@jiromaykin You've marked yourself as 'requested changes' here but I don't see any requests. Did you meant to just add a comment? |
Well I did :-) I asked for the unused colon to be removed (but putting in a useful title in there instead is also acceptable), so now I can approve. |
| ] | ||
|
|
||
| @admin.display( | ||
| description=_("Allowed CSS properties"), |
There was a problem hiding this comment.
This is what solved my issue in the request; approved.
Including @jiromaykin because I did some SCSS.