Opened 12 years ago
Closed 11 years ago
#18511 closed Cleanup/optimization (fixed)
Admin Site: Error Message displays different for Changeforms and the Password Change screen.
Reported by: | Serge Spaolonzi | Owned by: | Serge Spaolonzi |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | yes |
Description
In the Admin site the error message ('Please correct the errors below.') shows in different positions for the changeform screens and the password change screen.
In the Change Form screens it shows bellow the h1 title and in the password change screens it shows above the h1 title.
Change History (11)
comment:1 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 12 years ago
comment:3 by , 12 years ago
Summary: | Admin Site: Error Message displays differently for the Changeforms and the Password Change screen. → Admin Site: Error Message displays different for Changeforms and the Password Change screen. |
---|
follow-up: 5 comment:4 by , 12 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Cleanup/optimization |
Thanks for the report and patch. It would be more consistent with the rest of the codebase if the title was passed as a context variable from the view to the template. Could you make that change and also submit a pull request? Thanks!
comment:5 by , 12 years ago
Version: | 1.4 → master |
---|
Replying to julien:
Thanks for the report and patch. It would be more consistent with the rest of the codebase if the title was passed as a context variable from the view to the template. Could you make that change and also submit a pull request? Thanks!
Done.
I have made the changes in a new branch, the branch I posted before was based in 1.4, the new branch is base in master.
https://github.com/cobalys/django/tree/ticket_18511
follow-up: 7 comment:6 by , 12 years ago
Thank you, the patch looks good. However, I'm wondering, why have two different variables (content_title
and title
)? Can't we just have one that is used both in <title>
and <h1>
?
comment:7 by , 12 years ago
Replying to julien:
Thank you, the patch looks good. However, I'm wondering, why have two different variables (
content_title
andtitle
)? Can't we just have one that is used both in<title>
and<h1>
?
I had to use two variables because the value of <title>
and <h1>
are not always the same. In the template:
'django/contrib/admin/templates/registration/password_reset_confirm.html' title is always 'Password reset' but the value of 'h1' may be 'Enter new password' or 'Password reset unsuccessful' according to the situation. It was implemented that way and I tried to keep the same values.
follow-up: 9 comment:8 by , 12 years ago
Ok, I see, thanks for clarifying. I think this is a good opportunity to simplify things a bit by only having one title used both in the <h1>
and the <title>
. Could you merge those two variables into a single one and keep the title value that makes most sense in each case?
comment:9 by , 12 years ago
Replying to julien:
Ok, I see, thanks for clarifying. I think this is a good opportunity to simplify things a bit by only having one title used both in the
<h1>
and the<title>
. Could you merge those two variables into a single one and keep the title value that makes most sense in each case?
The code is updated with those changes, I left only one variable named 'title'.
comment:10 by , 12 years ago
I have modified the code, I have deleted an extra test that was unnecessary.
https://github.com/cobalys/django/tree/ticket_18511
comment:11 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Fixed.
https://github.com/cobalys/django/tree/ticket_18511_1_4
https://github.com/cobalys/django/blob/ticket_18511_1_4/django/contrib/admin/templates/registration/password_change_form.html