Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The implementation of our element alteration in Commerce Discount Date is quite poor. We're setting a title for the start date but not making it visible. We're hiding elements and implementing UI anti-patterns via CSS when we should just be altering elements to be non-#access-ible. This needs a complete re-implementation.
Comment | File | Size | Author |
---|---|---|---|
#15 | Edit_Life_Day_discount___Site-Install.png | 29.26 KB | joelpittet |
#12 | 2600030-12-interdiff.txt | 882 bytes | nvahalik |
#12 | 2600040-12-stop-hiding.patch | 8.77 KB | nvahalik |
#7 | seven-after.png | 18.62 KB | joelpittet |
#7 | seven-before.png | 24.24 KB | joelpittet |
Comments
Comment #2
rszrama CreditAttribution: rszrama at Centarro commentedGo, go, gadget testbot!
Comment #4
rszrama CreditAttribution: rszrama at Centarro commentedBah, apparently the Date module is very touchy when it comes to the start_todate element. I'm sure there's a fix, but it doesn't lie in hiding the element via #access or a simple conversion of the element to a value unfortunately.
Comment #5
joelpittetYeah this is a pain. Took quite some time to wrangle this into shape but I think I got something...
Will take screenshots if it passes.
Comment #6
joelpittetHere's a before and after in my admin theme (adminimal)
Comment #7
joelpittetI removed the fieldset styling as I figure the admin theme can do a good job at that. My goal is to remove as much custom styling as possible.
Seven Before
Seven After:
Comment #8
joelpittetI can add some better date icons back if you want, but those ones were pretty gross. And inline the labels still if you want. But would rather remove styles than add them in this case.
Comment #9
nvahalik CreditAttribution: nvahalik at Centarro commentedJust adding a note that this patch will end up causing a conflict with the date/usage merge patch.
Comment #12
nvahalik CreditAttribution: nvahalik at Centarro commentedReroll of the patch. Adding vertical tabs also introduced a slight regression where the first field date text field was 5px lower than the second one. So this patch also fixes that by removing the extra 5px of padding.
Comment #14
joelpittetThanks @nvahalik I've committed this with a calendar icon replaced with SVG from http://evil-icons.io/.
We can change the icon if there is a decision made #2625938: Replace image icons with SVG for HiDPI devices for commerce but I feel that is better than what we had.
Comment #15
joelpittetFor the visual minded: here's a screenshot.