Problem/Motivation

The office_hours.js hide every add button on the page

Steps to reproduce

1. install office_hours
2. install Inline Entity Form or any other module that add Add button
3. Add field reference for exemple field_ref refer to page content and choose the widget Inline entity form - Complex with the option "Allow users to add new nodes."
4. Add office hours field

5. Go to Add article form page:

The button to add new content is hidden because the office_hours.js add this code $('[id$=add]').each(showAddLink); which hide every button with ID ends with add.

Article add page with office hours
screen with office hours

Article add page without office hours
screen with office hours

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#18 office_hours_3422607-17_javascript.patch6.14 KBjohnv
#10 office_hours-fix_hides_other_add_btns-3422607_10.patch1.87 KBberramou
#2 office_hours-fix_js_hide_all_add_buttons-3422607-1.patch512 bytesberramou
Screenshot-without_office-hours.png73.84 KBberramou
Screenshot-with_office-hours.png212.49 KBberramou
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

berramou created an issue. See original summary.

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented
FileSize
office_hours-fix_js_hide_all_add_buttons-3422607-1.patch512 bytes

Patch that fix it for, it still needs works to target only add button added by the office hours module and not target other buttons in the page.
Maybe look at the other selectors like $('[id$=copy]'), $('[id$=clear]') to not apply those callbacks to other elements that added by other modules/themes with ids ends with copy, clear, or add.

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented
Priority: Major » Critical
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
Title: office_hours.js hide other add buttons » office_hours.js hides other [Add] buttons in widget
Status: Active » Needs work

This needs more investigation.
The proposed solution breaks the 'add' link usage in the office_hours widget.

The better solution must restrict the scope for (amongst others) the following lines. Only the id's within a certain DIV must be regarded:

        $('[id*="all-day"]').bind('click', setAllDayTimeSlot);
        $('[id$=add]').bind('click', addTimeSlot);
        $('[id$=add]').each(showAddLink);
        $('[id$=clear]').bind('click', clearTimeSlot);
        $('[id$=copy]').bind('click', copyPreviousDay);
berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented

Thank you @johnv for your quick reply, and yes it should be scoped within specific element like a DIV.

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

I am sorry I cannot help you with that.
As a maintainer, I only copy&paste&test the js code.

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented

Thank you for your reply @johnv

The js is in the module and it breaks the other modules features so it should be fixed in the module, otherwise every one want to use the module will patch it.

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

Indeed, it should be fixed in this module.
My point was that I do not have the knowledge to do that in a limited timeframe. For that, I need help from contributors.

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

Having said that, the second best option is to replace all binded/bound link-ID's (as listed below) and change the ID in js and php to e.g., 'office-hours-clear', with a comment in the php file as to why the ID is so long.

$('[id*="all-day"]').bind('click', setAllDayTimeSlot);
$('[id$=add]').bind('click', addTimeSlot);
$('[id$=clear]').bind('click', clearTimeSlot);
$('[id$=copy]').bind('click', copyPreviousDay);
berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented
FileSize
office_hours-fix_hides_other_add_btns-3422607_10.patch1.87 KB

Here is a path that wrap all office hours operations buttons in a specific DIV and then in the JS only target it's OP buttons.

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented
Status: Needs work » Needs review
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

Does this issue also solve the problem? #3424112: Unable to add, remove or copy timeslot

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented

Yes @johnv #3424112: Unable to add, remove or copy timeslot is duplicate of this issue, and yes this fixes the other issue too.

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

Thanks,
Actually, my question was the other way around: Does the solution of issue 3424112 also solve your problem from 3422607?
I like the other approach a bit better, since it does not add the extra DIV.

skyriter’s picture

skyriter CreditAttribution: skyriter at CivicActions for Department of Veterans Affairs commented

#10 worked for me, but the other 3424112-1.patch on Unable to add, remove or copy timeslot did not.

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

That would be because the drupal selector is common for all drupal modules of course. The name or the div should be unique for this module only.

  • johnv committed 45ef1ca6 on 8.x-1.x 
    Issue #3422607: office_hours.js - better sorting of functions
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
FileSize
office_hours_3422607-17_javascript.patch6.14 KB

Adding explicitly a more specific (but not unique) 'id' should do the trick. See patch.

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented

I don't think it's a good idea, if we have multiple fields of office hours, you will have more than one identical ID within the same HTML page, which is not recommended.
Apply the patch and create 2 fields of type office hours in the same content type like article, you will have multiple html elements with the id office-hours-add

  • johnv committed 7115ce00 on 8.x-1.x authored by berramou 
    Issue #3422607 by berramou, jonas139, johnv, skyriter: office_hours.js...
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
Status: Needs review » Fixed

ndeed, non-unquye ID's are not preferred.
I committed #10 with slight changes.
I tried to re-use the 'office-hours-link' class that already exists, but did not succeed.

Thanks all for your detailed initial postings, the fast responses and the contributions.
I will wait 48 hours before creating a new release.

berramou’s picture

berramou CreditAttribution: berramou as a volunteer commented

Thank you all for your reactivity, support, and contribution.

skyriter’s picture

skyriter CreditAttribution: skyriter at CivicActions for Department of Veterans Affairs commented

johnv, do you know when a new release will be created?

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented
johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

I just created a new version.

skyriter’s picture

skyriter CreditAttribution: skyriter at CivicActions for Department of Veterans Affairs commented

Terrific. Thank you!

jonas139’s picture

jonas139 CreditAttribution: jonas139 at iO commented

I haven't reviewed the changes yet but my colleague has tested the latest release and we are having the same issue from https://www.drupal.org/project/office_hours/issues/3424112 again.

A new issue has been created which mentions the same issue I had: https://www.drupal.org/project/paragraphs/issues/3426406.

johnv’s picture

johnv
Primary language Dutch
CreditAttribution: johnv commented

Are you using Paragraphs module like described in #3426406: Office_hours Action links always point to frontpage on uncollapsed paragraphs ? (I just finished enriching that issue).
office_hours should now be affecting only the own elements.

I think it just depends on some sort order (alphabetically module name?) to tell what modules affects what.

Status: Fixed » Closed (fixed)

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