Problem/Motivation

When a theme is disabled its breakpoint.breakpoint_group.theme.%themename.yml configuration file, if it has one, is removed from the active configurations, but its corresponding picture.mappings.%themename.yml, if it has one, is not.

Proposed resolution

Decide if this expected behavior.

Remaining tasks

If this is expected, provide documentation.
If not, provide a patch.

Comments

attiks’s picture

attiks CreditAttribution: attiks commented

FYI: picture is renamed to responsive_image, see #2124377-74: Rename "Picture" module to "Responsive Image" module

dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented

I'll update once that patch is committed. I haven't got any feedback on this yet. Since any 'responsive_image' file can be used by any theme, I'm not sure if this isn't 'as designed'.

Eli-T’s picture

Eli-T
he/him
Primary language English
Location Manchester
CreditAttribution: Eli-T commented
Component: picture.module » responsive_image.module
dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented
Title: Picture mapping yml file not removed when theme is disabled » respomsive_image mapping yml file not removed when theme is disabled
dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented
Title: respomsive_image mapping yml file not removed when theme is disabled » responsive_image mapping yml file not removed when theme is disabled

Sorry, typo.

dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented
Title: responsive_image mapping yml file not removed when theme is disabled » responsive_image mapping doesn't implement 'delete' function

It appears that the responsive_image form ui omits a 'delete' functionality, even though 'ResponsiveImageMappingDeleteForm.php' exists. The test in 'ResponsiveImageAdminUiTest.php' doesn't actually delete the temporary mapping, and there is no test to see if it did so successfully. It appears that the implementation of the delete functionality wasn't completed. Was this intentional?

attiks’s picture

attiks CreditAttribution: attiks commented
Status: Active » Postponed

It was not intentional, so it should be added. I think the easiest is to do it as part of #2260061: Responsive image module does not support sizes/picture polyfill 2.2 or at least wait till that gets committed.

dcrocks’s picture

dcrocks CreditAttribution: dcrocks commented

Probably better to keep it here. Right now mappings can be used by any theme thru the UI. Maybe that shouldn't be so.

attiks’s picture

attiks CreditAttribution: attiks commented
Status: Postponed » Closed (duplicate)

This is part of the latest patch in #2260061: Responsive image module does not support sizes/picture polyfill 2.2