Fixed #35489 -- Fixed vertical alignment of raw_id_fields widget.
In the modeladmin, when using raw_id_fields = ("parent",) in the ModelAdmin, the search button and name are aligned at the top.
This PR fixes that allign.
Before
After
Hello @Atem18, thank you for your interest in making Django better.
While I agree that the search button and corresponding text look better when they are center-aligned, the change you are proposing is affecting many other admin pages and pieces. For example, see the User detail page (see how the field labels change from top aligned to centered):
-
In
main: -
In this branch:
@nessita Hi, thanks for reviewing my PR. For me it looks great to have them centered instead of top-aligned because when you look at other smaller fields, they looks like aligned in the center and not the top:
I guess this should be fixed anyway because in forms.css, the .related-lookup class has the property vertical-align: middle; but it does not work and Firefox recommends to fix it:
Hello @Atem18!
I consulted with a Django contributor who has worked recently on the admin CSS and that is also part of the accessibility team, and they laid our a few points I agree with:
- Labels aligned at the top are better for scanning a page top-bottom. If there is a big fieldset like the one for permissions, on smaller screens a user may see the field but not the label and that could be confusing.
- The alignment of text/search icon with the label is nice but we don't think it's worth it for the regression on larger fields.
Perhaps it would be better to align the input text better in a top aligned mode though it may not be worth the effort. Could you think of another way of aligning that icon?
Hi @nessita, thanks a lot for the comments, I agree with it indeed.
As for the lookup, they were properly aligned in at least the version 4.1.7 of Django. Before, they were not wrapped in the flex-container and it looked just nearly like my "after" screenshot:
I can do some tests to see in which version this changed but maybe the person who worked on it will know quicker than I do.
(this isn’t an accessibility issue so I’ve untagged the team as reviewers. Tom has volunteered himself instead)
Same thing for the ForeignKey.
In 4.1.7:
In 5.0.1:
I can confirm that it changed in the version 5.0, it was fine in the 4.2.9
diff --git a/django/contrib/admin/static/admin/css/forms.css b/django/contrib/admin/static/admin/css/forms.css
index 539a11ae61..be1a1b8174 100644
--- a/django/contrib/admin/static/admin/css/forms.css
+++ b/django/contrib/admin/static/admin/css/forms.css
@@ -491,7 +491,8 @@ body.popup .submit-row {
.related-lookup {
margin-left: 5px;
display: inline-block;
- vertical-align: middle;
+ margin-top: auto;
+ margin-bottom: auto;
background-repeat: no-repeat;
background-size: 14px;
}
For me, something like this works.
We need to check if other instances with vertical-align: middle; need to be updated 👍
@Atem18 are you planning to submit an updated patch? I didn't want to assume since it's been a few months since this ticket was active.
@samGbos I can but I don't know if the solution by @sarahboyce is acceptable. If it is yes, then for sure I update my PR. :)
Hey @Atem18 I have tried out the solution suggested by @sarahboyce. Are you planning to update your PR with that solution?
Hey @Atem18 I have tried out the solution suggested by @sarahboyce. Are you planning to update your PR with that solution?
Ok, I will do it then, I was waiting on someone giving me the go to do it.
Hey @Atem18 have you had the time to update your PR?
Hi @VaarunSinha, not yet, I hope to do it this week.
Hey @Atem18 have you had the time to update your PR yet?
Hi @VaarunSinha Not yet, I had a busy weekend and will have a busy week + I am sick. Let's see if I can advance on this for this week.
I installed the latest Django from git and the bug is gone, it is properly aligned again.
Got it thank you for your time and contribution anyway! Get well soon!
@VaarunSinha @Atem18 I don't think this is fixed in latest Django, this is still an issue in main so if anyone wants to to pick this up, please assign the ticket to yourself. Thank you everyone!
I have a fix ready I will create a PR today.
Created a PR : https://github.com/django/django/pull/18344