Code

Opened 22 months ago

Closed 9 months 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: master
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.

Attachments (0)

Change History (11)

comment:1 Changed 22 months ago by serge_spaolonzi

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to serge_spaolonzi
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:3 Changed 22 months ago by serge_spaolonzi

  • Summary changed from Admin Site: Error Message displays differently for the Changeforms and the Password Change screen. to Admin Site: Error Message displays different for Changeforms and the Password Change screen.

comment:4 follow-up: Changed 22 months ago by julien

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to 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 in reply to: ↑ 4 Changed 22 months ago by serge_spaolonzi

  • Version changed from 1.4 to 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

comment:6 follow-up: Changed 22 months ago by 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>?

comment:7 in reply to: ↑ 6 Changed 22 months ago by serge_spaolonzi

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 follow-up: Changed 22 months ago by 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?

comment:9 in reply to: ↑ 8 Changed 22 months ago by serge_spaolonzi

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 Changed 21 months ago by serge_spaolonzi

I have modified the code, I have deleted an extra test that was unnecessary.
https://github.com/cobalys/django/tree/ticket_18511

comment:11 Changed 9 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In e07e4030b9170d522a9670a80c4fd40acff369cb:

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.