Updated until comment #31
Problem/Motivation
Maintaining hours of many locations can be tedious with the current widget. It needs some UX improvements.
Proposed resolution
I made a feature enhancement that adds links to the end of each day's hours which allow you to do the following quickly:
- Clear the hours of the line item; --> Fixed in this issue
- Apply First day's opening and closing hours to the other days of the week --> handled in #2872213: Add 'same as above' to widget to improve usability
This functionality mimics Google places office hours widget.
The attached patch only works fine if only 1 block per day is allowed.
Remaining tasks
Please adjust to the following case:
- set the field settings to having 4 blocks per day.
- in the widget, add 2 blocks.
- now press 'Apply to all'. You'd expect to the 2 blocks to be copied to all other days. Instead, the first line is copied to every other line.
Correction of this may need another level in the widget (adding Day between Week and Block) or some algorithm using the DayDelta.
Comment | File | Size | Author |
---|---|---|---|
#28 | office_hours_2044465_27_copy_link.patch | 3.07 KB | johnv |
Comments
Comment #1
badjava CreditAttribution: badjava commentedComment #2
aze2010 CreditAttribution: aze2010 commenteddoesnt work with stable version 1.3
Comment #3
badjava CreditAttribution: badjava commentedThe patch was for the dev version.
Comment #4
mnshantz CreditAttribution: mnshantz commentedI tried using the patch on the dev version and clicking either Closed or Apply to All would redirect me to the sites homepage.
Comment #5
johnvbetter status.
Comment #6
badjava CreditAttribution: badjava commentedDid you flush your cache? The patch changes the javascript file which it sounds like is not getting fired.
Comment #7
yannickooThe error in debugger is
Uncaught TypeError: Object #<Object> has no method 'prop'
Comment #8
mnshantz CreditAttribution: mnshantz commented#6 - Yes, I had cleared caches and still had the issue.
Comment #9
yannickoo.prop() is not available in Drupal's jQuery version so I rewrote the JavaScript code and BTW there were some whitespaces which are gone now.
Comment #10
yekezei CreditAttribution: yekezei commentedclicking apply to all resets the entire form for me
Comment #11
johnvThe patch needs rerolling (I can't get the JS working myself.)
- The 'closed' link should be renamed to 'clear hours'.
- The 'copy to all' should be refactored to 'copy to next day' ?
A 'closed' checkbox is requested here: #2070145: Add option to display 'Closed all week'
Comment #12
yannickooWill work on this tomorrow :)
Comment #13
yannickooNot as easy as I thougt, unassigning :/
Comment #14
seworthi CreditAttribution: seworthi commentedI modified the jquery to work with the latest -dev version of the module. You need to find both the correct field name and id for the field (which has language code in it). Issue with previous one is it "assumed" field name, and did not work if more that one "office hours" field on same form.
Comment #16
johnvAbove commit has nothing to do with this issue.
Comment #18
johnvLet's take this slowly and one at a time.
Above commit adds the 'Clear this line' link. I named it 'Remove', Since 'Clear' has no default translations, and 'Clear/Remove/Delete this line/item/thingy' is too long.
As stated in #11, 'Closed' is not the correct wording.
Comment #19
johnvI combined the remainder of the patches of @yannickoo and @seworthi , nd the attached patch is the result. It works fine, but not good enough.
Please adjust to the following case:
- set the field settings to having 4 blocks per day.
- in the widget, add 2 blocks.
- now press 'Apply to all'. You'd expect to the 2 blocks to be copied to all other days.
Comment #20
johnvP.S. Hiding the 'clear' link if the block is empty would be nice.
Comment #21
Dom. CreditAttribution: Dom. commentedHi!
I'm french, thus my website is configured with monday being first day of the week (ISO 8601). But the "Apply to all" link is added on Sunday, thus it appears on the last day of the configuration form. How can this be solved to add on first day of week ?
Dom.
Comment #22
Dom. CreditAttribution: Dom. commentedHi,
Coming back with very easy solution :
Change
to
Do you need a patch for this ?
Dom.
Comment #23
Dom. CreditAttribution: Dom. commentedThere is also a bug in the "remove" link : hours keeps at "00" instead of being deleted.
- Nice but hard way : recode the creation of hours to have value="00" in hours just as per minutes.
- Easy fix way: change line 20 of office_hours.js from
$(this).val(0);
to
$(this).val($("#target option:first").val());
Also please implement correction as per johnv #19 use case : duplicate multi-block days.
Dom.
Comment #24
johnv@Dominique CLAUSE,
I've created #2398691: 'Apply to all' link on Widget when Monday is first-day-of-week for your bug report #21-22. A patch is not necessary, but a new issue next time would be nice!
Comment #26
johnv@Dominique CLAUSE, #23 is now committed. (For some reason I cannot attribute you, since your name seem tied to another user.)
Comment #27
johnvComment #28
johnvA fresh reroll of the patch.
Comment #29
johnvComment #30
Dom. CreditAttribution: Dom. as a volunteer and at ACINO commented@johnv regarding #26: you can credit me via @Dom.
Comment #31
abrlam CreditAttribution: abrlam commentedAnother approach to 'apply to all' is 'same as above'.
See issue #2872213 for details.
Comment #32
johnvThis issue is now closed.
The feature 'copy to all' will be implemented using 'copy from previous day' in #2872213: Add 'same as above' to widget to improve usability