I cloned the default scald_dnd_library view, made some modifications and assigned my new view to be used by the Scald in the sidebar. Noticed that the reset button wasn't working when enabled through the views UI for the exposed filters. It seems the markup generated by views for the reset button isn't what dnd_library.js is expecting; dnd_library.js seems to be expecting (logically!) that the input would be type=reset but it's type=submit button and thus was acting as a submit button based on the code in dnd_library.js.

Markup generated by views with Reset button enabled:

<div class="views-exposed-widget views-submit-button">
  <input type="submit" id="edit-submit-media-library" name="" value="Apply" class="form-submit">
</div>
<div class="views-exposed-widget views-reset-button">
  <input type="submit" id="edit-reset" name="op" value="Reset" class="form-submit">
</div>

Notice that both are type=submit. Don't know if this is something to report to views, but would find it less likely to be changed that just making the change here. I fixed some indention for readability, so the patch looks like I redid the whole file, but the gist of the patch is this, on lines 319 and 337 in dnd_library.js (280 and 319 in the patch):

-  $('.view-filters input[type=submit]', $this).click(function(e) {
+  $('.view-filters .views-submit-button input', $this).click(function(e) {

-  $('.view-filters input[type=reset]', $this).click(function(e) {
+  $('.view-filters .views-reset-button input', $this).click(function(e) {

Not certain, but for the same reason, line 98 may need to be changed to be more specific too? I didn't make this change but may be something to consider based on the above.

$('.dnd-library-wrapper .view-filters input[type="submit"]').click();

Any feedback is welcome. Thx for all you do!

CommentFileSizeAuthor
#14 scald-views-reset-button-2215761-14.patch1.17 KBscotwith1t
#13 2215761-reset-button-13.patch1.07 KBjcisio
#7 scald_views_reset.patch16.19 KBscotwith1t
#4 scald_views_reset.patch16.29 KBscotwith1t
scald_views_reset.patch16.29 KBscotwith1t
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, scald_views_reset.patch, failed testing.

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t commented

No idea how to interpret the bot's results. Any help is appreciated. :)

DeFr’s picture

DeFr CreditAttribution: DeFr commented

@scotself: The patch you provided is relative to your Drupal root ; the testbot for contrib modules is assuming path relative to the module root, and thus patching fail :-)

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t commented
Status: Needs work » Needs review
FileSize
scald_views_reset.patch16.29 KB

Maybe this will work.

DeFr’s picture

DeFr CreditAttribution: DeFr commented

It won't.

Let me clarify what I tried to convey before using concrete things: the patch starts with 

diff --git a/sites/all/modules/scald/modules/library/dnd/js/dnd-library.js b/sites/all/modules/scald/modules/library/dnd/js/dnd-library.js
index 5f8abff..8861694 100644
--- a/sites/all/modules/scald/modules/library/dnd/js/dnd-library.js
+++ b/sites/all/modules/scald/modules/library/dnd/js/dnd-library.js

Instead it should start with

diff --git a/modules/library/dnd/js/dnd-library.js b/modules/library/dnd/js/dnd-library.js
index 5f8abff..8861694 100644
--- a/modules/library/dnd/js/dnd-library.js
+++ b/modules/library/dnd/js/dnd-library.js

Status: Needs review » Needs work

The last submitted patch, 4: scald_views_reset.patch, failed testing.

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t commented
Status: Needs work » Needs review
FileSize
scald_views_reset.patch16.19 KB

I created the patch from within the Scald folder, so don't know what to do except manually update those paths. Let's try this again.

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t commented

Hooray! I guess I won't mark my own RTBC, so I'll let someone check it out.

tedstein’s picture

tedstein CreditAttribution: tedstein commented
Status: Needs review » Reviewed & tested by the community
tedstein’s picture

tedstein CreditAttribution: tedstein commented

Will this be included in your next release?

jcisio’s picture

jcisio CreditAttribution: jcisio commented

7: scald_views_reset.patch queued for re-testing.

jcisio’s picture

jcisio CreditAttribution: jcisio commented
Version: 7.x-1.1 » 7.x-1.x-dev
Status: Reviewed & tested by the community » Needs work

Patch does not apply here. Could you create a smaller patch without unrelated changes (code indent...). From #0 there are only two lines to be changed. And patch must be rolled against the latest dev, not 1.1.

jcisio’s picture

jcisio CreditAttribution: jcisio commented
FileSize
2215761-reset-button-13.patch1.07 KB

Here is the minimal patch rerolled against 1.1. But it does not apply on 1.x.

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t commented
Status: Needs work » Needs review
FileSize
scald-views-reset-button-2215761-14.patch1.17 KB

Re-rolled against 1.x-dev

  • Commit 7798546 on 7.x-1.x by jcisio: 
    Issue #2215761 by scotself, jcisio: Reset button on views exposed...
jcisio’s picture

jcisio CreditAttribution: jcisio commented
Status: Needs review » Fixed

I really don't like to add a new Views-specific class into dnd, but because: 1/ it is a bug 2/ it is the simple way to fix that 3/ there are already many Views-specific classes, so let's add another one.

Patch committed and pushed. Thanks scotself.

scotwith1t’s picture

scotwith1t
Location Birmingham, AL
CreditAttribution: scotwith1t commented

Awesome. I figure that views will always be used to build the dnd library, so it makes perfect sense to use the views classes. I bet the class names don't even change in D8 with views in core. Thanks!

Status: Fixed » Closed (fixed)

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