Opened 3 years ago

Last modified 2 years ago

#21987 assigned Cleanup/optimization

Allow Media objects to have their own MEDIA_TYPES

Reported by: Keryn Knight <django@…> Owned by: Osmose
Component: Forms Version: master
Severity: Normal Keywords:
Cc: hirokiky@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


Currently, instances of Media look to the module scope to get the known MEDIA_TYPES. This makes it difficult to subclass Media to add extra render_x/add_x methods because MEDIA_TYPES is hardcoded in __init__, render, __getitem__ and __add__ and isn't easily available for a subclass to add to without monkeypatching the module constant. As far as I can tell, the only use of MEDIA_TYPES is in the widgets module, and could be shifted to the class itself, leaving the module level one to raise a deprecation warning.

Rationale is that I want to be able to add inline_css and inline_js, or text (for JS templates etc) to a Media object, and currently that's more difficult than just adjusting a subclass' attribute and adding the appropriate methods.

Change History (6)

comment:1 Changed 2 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 2 years ago by aaugustin

See also #13978.

comment:3 Changed 2 years ago by Osmose

  • Has patch set
  • Owner changed from nobody to Osmose
  • Status changed from new to assigned

comment:4 Changed 2 years ago by hirokiky

  • Cc hirokiky@… added
  • Patch needs improvement set

this patch may need improvement.
you should consider the case when combining CustomMedias which has different MEDIA_TYPES,
see more

comment:5 Changed 2 years ago by Osmose

Good point! Either throwing an exception on combining Media subclasses with different Media types, or throwing one when there's a name conflict in the MEDIA_TYPES list sound like good options to me; I'd probably lean towards the latter simply because it's more flexible and not really that complex to add.

However, I'm hesitant to make changes to the patch before #22298 is resolved, as if form media is going away, improvements aren't terribly relevant anymore. (I submitted the patch before someone pointed that issue out to me, of course :P).

comment:6 Changed 2 years ago by hirokiky

@Osmose thanks for your reply. the after one sounds greater! > throwing one when there's a name conflict in the MEDIA_TYPES list

#22298 discussion looks hassle a little (´Д`), the patch will probably have to wait for a while.
But you've done a great job. im pretty sure.

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