#34995 closed Bug (fixed)
Add link for admin's related widget is in the wrong place
Reported by: | Tom Carrick | Owned by: | Tom Carrick |
---|---|---|---|
Component: | contrib.admin | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description (last modified by )
Attachments (1)
Change History (16)
by , 12 months ago
Attachment: | Screenshot 2023-11-23 at 16.01.04.png added |
---|
comment:1 by , 12 months ago
Description: | modified (diff) |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 12 months ago
Version: | 4.2 → 5.0 |
---|
Regression in 1699f8b52ac15f687cc39088401c2641022c71cd.
So I think this is a release blocker for 5.0, not 4.2?
comment:4 by , 12 months ago
Triage Stage: | Unreviewed → Accepted |
---|---|
UI/UX: | set |
Agreed, this is a 5.0 release blocker.
comment:5 by , 12 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:6 by , 12 months ago
Triage Stage: | Ready for checkin → Accepted |
---|
Getting a selenium failure that needs debugging.
follow-up: 8 comment:7 by , 12 months ago
Severity: | Release blocker → Normal |
---|
IMO we shouldn't treat this as a release blocker. "+" always had various issues, you can play with font and screen sizes and you'll notice various incorrect behaviors in the older versions of Django. I don't believe that Django 5.0 made it significantly worse. Also, there is a huge risk that the proposed patch will introduce further regressions. Let's don't backport it.
comment:8 by , 12 months ago
Replying to Mariusz Felisiak:
IMO we shouldn't treat this as a release blocker. "+" always had various issues, you can play with font and screen sizes and you'll notice various incorrect behaviors in the older versions of Django. I don't believe that Django 5.0 made it significantly worse. Also, there is a huge risk that the proposed patch will introduce further regressions. Let's don't backport it.
I leave the final decision to Natalia, I'm not going to die on this hill.
comment:9 by , 12 months ago
Personally, I don't think this needs to be in 4.2.x. However, I would like to see it in 5.0. I promise to fix any regressions!
comment:12 by , 12 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
I have merged the PR #17517 in main
and will review how a potential backport would look like in stable/5.0x
. Then I'll report back on next steps. Thanks everyone!
comment:13 by , 12 months ago
I have locally backported and tested 999ba9db6d6331eaa58af77debba42754bcc1a8e in stable/5.0.x
and I haven't find any (new) Admin UI issues, only improvements. I have tested small (phone), medium (tablet) and wide (desktop) screens, both horizontal and vertical rotated viewports, light and dark modes, and LTR and RTL languages.
I'm a bit dizzy at this point but I'm fairly confident that with this change, the "green plus sign" is consistently shown to the right of the M2M selector (in LTR langs, it's shown to the left in RTL langs) and I think this is an improvement over what's currently available.
Given the above, and given Tom's explicit commitment to fix any potential regression, I think this is a good change to be included in 5.0. I'll be backporting the mentioned revision and the previous one that make some Selenium tests more robust.
[PR https://github.com/django/django/pull/17517]