In order to overcome the issue of a stylesheet not being generated when the configuration changes, as highlighted in point 4 of this comment #3114980-7: Add some automated tests
The creation of the stylesheet should be moved to an event subscriber instead. This should function as a good start for creating one: https://www.drupal.org/docs/8/creating-custom-modules/subscribe-to-and-d...
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff_5-7.txt | 300 bytes | hkirsman |
| #7 | high_contrast-stylesheet-event-subscriber-3115713-7.patch | 4.86 KB | hkirsman |
| #5 | interdiff_2-5.txt | 2.03 KB | hkirsman |
| #5 | high_contrast-stylesheet-event-subscriber-3115713-5.patch | 4.79 KB | hkirsman |
| #2 | 3115713-stylesheet-event-subscriber-2.diff | 3.41 KB | neograph734 |
Comments
Comment #2
neograph734Something like this I think..
Comment #4
neograph734Comment #5
hkirsman commentedI haven't worked with events but I see this is pretty cool.
I tested with drush cim and worked nicely.
I also see that it's good opportunity get rid of some code duplication. We don't need install hook anymore as on module install the configuration is created and hook is triggered. We also don't need to duplicate the css saving in update hook because if we trigger configuration save, css is also saved.
Adding patch and interdiff.
PS: I noticed there's no uninstall hook so the css is left behind. Added task here https://www.drupal.org/project/high_contrast/issues/3116364
Comment #7
hkirsman commentedLast build failed as the use is not needed in install file anymore. It might be that it still fails - not sure if it's looking for tests? Adding patch.
Comment #9
neograph734The 'unused use' is just a coding standards message. It probably failed because there are no tests.
But the code looks good to me.