[Django] #32771: Hardcoded _popup values should use IS_POPUP_VAR instead
Django
------------------------------------------------+------------------------
Reporter: David Sanders | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
There's a handful of places in the codebase which hardcode the '_popup'
string rather than using the `IS_POPUP_VAR` constant. They should all use
`IS_POPUP_VAR` for consistency.
I've got a PR to fix all occurrences other than two cases in the admin
JavaScript, since there's not a clear fix there. Possible solutions for
JavaScript would be to add it into the base admin template and pull it
back out in the code that needs it, or hardcode it in a single spot in the
JavaScript on `window`. Neither solution feels great to me, so I'm kicking
the can down the road on the JavaScript occurrences.
--
Ticket URL: <https://code.djangoproject.com/ticket/32771>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: David Sanders | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
Comment:
I couldn't find many places where '_popup' is used instead of
`IS_POPUP_VAR` (except tests) but I'd be happy to review a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/32771#comment:1>
Django
Reporter: David Sanders | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: 3.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Sanders):
@Mariusz, yea, outside of tests it's only used in two templates, and two
JavaScript files. I've just spent a lot of time in the
`django.contrib.admin` code in the last month so I've got a list of PRs
I'm checking off for cleanup/optimization of things I noted while in the
code. There's a PR for this on GitHub now.
--
Ticket URL: <https://code.djangoproject.com/ticket/32771#comment:2>
Django
Reporter: David Sanders | Owner: nobody
Component: contrib.admin | Version: 3.2
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"2978c63a34b4aa0f170a1e5a0f8f4cb2811fa248" 2978c63]:
{{{
#!CommitTicketReference repository=""
revision="2978c63a34b4aa0f170a1e5a0f8f4cb2811fa248"
Fixed #32771 -- Used IS_POPUP_VAR constant instead of hard-coded value.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32771#comment:3>