Problem/Motivation

The current behavior of all day widget is to use a checkbox as UI improvement, managed by js, to set the time from 00:00 to 23:59.
The javascript code assumes there is some markup provided by seven, so it does not work on other admin themes, for example Claro theme.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 3192945-2-support-claro-theme.patch632 bytesericmaster

Issue fork date_all_day-3192945

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

akalam created an issue. See original summary.

ericmaster’s picture

ericmaster commented
StatusFileSize
new 3192945-2-support-claro-theme.patch632 bytes

Just needed this for Gin admin theme, not sure if it will work with the Claro theme, and there is probably a better approach, but here is a patch that worked for me.

ericmaster’s picture

ericmaster commented
Status: Active » Needs review

tunic made their first commit to this issue’s fork.

tunic’s picture

tunic
he/him
Primary language Spanish
Location Madrid
commented

Thansk for the patch @Ericmaster.

However, I think the approach means adding a new jQuery selector to support themes that use different CSS classes. I've modified the jQuery selecto to be more general instead of more specific. This way I think we can support more themes with less code.

See issue fork.

I'd like some testing before committing this, specailly on already runnig sites with this module.

akalam’s picture

akalam commented

Thanks @tunic for the update.
You are changing the element on which the selector applies (from the fieldset wrapper to the fieldset itself). Anyway, since the parent selector is used in conjunction with the find() jQuery method I don't expect to have any drawback with this change.
There is a minor codding standard issue introducing a empty line with spaces. This is not a problem, I can fix it by myself while testing.
I'll test it this afternoon with seven, claro and adminimal

tunic’s picture

tunic
he/him
Primary language Spanish
Location Madrid
commented

Oops, the IDE installaton is new and I dind't configure automatic traling spaes remove on save.

Added a new commit to fix it.

Thanks for the review!

For the record, I've tested it with the themes available on my environment: Radix, Bartik, Claro, Olivero, Seven and even Stark. It worked even for the non-admin themes. Also, tested with single and multiple value fields (the multiple value fields are the ones that need the fieldset selector part I guess).

akalam opened merge request !2

  • akalam committed 7e989c7 on 8.x-1.x 
    Issue #3192945 by tunic, Ericmaster, akalam: Support Claro theme and...
akalam’s picture

akalam commented

Thanks @ericmaster and @tunic! I'm sorry for "stealing" the commit ownership. I checked the option to "squash" the commits on gitlab to avoid 2 commits because of the trailing white-space and it made me author automatically, sorry for that.
The issue credits are untouched.

akalam’s picture

akalam commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.