Opened 13 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 Serge Spaolonzi, 13 years ago

Owner: changed from nobody to Serge Spaolonzi
Status: newassigned

comment:3 by Serge Spaolonzi, 13 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.

comment:4 by Julien Phalip, 12 years ago

Has patch: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/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!

in reply to:  4 comment:5 by Serge Spaolonzi, 12 years ago

Version: 1.4master

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

comment:6 by Julien Phalip, 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>?

in reply to:  6 comment:7 by Serge Spaolonzi, 12 years ago

Replying to julien:

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>?

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.

comment:8 by Julien Phalip, 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?

in reply to:  8 comment:9 by Serge Spaolonzi, 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 Serge Spaolonzi, 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 Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In e07e4030b9170d522a9670a80c4fd40acff369cb:

Fixed #18511 -- Cleaned up admin password reset template titles.

Note: See TracTickets for help on using tickets.
Back to Top