Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Elijah Lynn’s picture

Status: Active » Needs review
FileSize
1.24 KB

Status: Needs review » Needs work

The last submitted patch, 1: drupal-views-duplicate-dropdowns-var-2302515-1.patch, failed testing.

Elijah Lynn’s picture

Status: Needs work » Needs review
FileSize
1.24 KB

Ahh, this patch #2301953: Update 'element-invisible' and 'element-hidden' to valid Drupal 8 CSS classes just got applied causing it to fail. Re-rolled.

Elijah Lynn’s picture

LinL’s picture

droplet’s picture

Status: Needs review » Needs work

I think it's more than "dropdowns"

http://cgit.drupalcode.org/drupal/tree/core/modules/views_ui/js/views-ad...

"titleRow" and some other expressions.

Elijah Lynn’s picture

Yeah, there are a bunch more in that file but I wasn't sure if I should make separate issues for each one?

droplet’s picture

that's fine to clean up in same issue, we did a lot of these cleanups before: #1415788: Javascript winter clean-up

nod_’s picture

Issue tags: +Novice
camoa’s picture

This should move the cleanup further.

I think that will do it as a clean up, there is some repeated code but it is not repeated functionality.

camoa’s picture

Status: Needs work » Needs review

droplet’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
Alka Kumari’s picture

Assigned: Unassigned » Alka Kumari
Issue tags: +drupalconasia2016
kostyashupenko’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
675 bytes

Re-roll of patch #10

andypost’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

still needs proper re-roll

kostyashupenko’s picture

Not sure that we still need re-roll, because there was an auto merge. Better to make a patch

gvso’s picture

If we remove dropdowns = this.operator, drow dropdowns = dropdowns.add(fakeOperator); will produce an error.

eleleka’s picture

Assigned: Alka Kumari » Unassigned
Status: Needs work » Needs review
FileSize
1.21 KB

Rerolled patch #10
There is some confusion in line numbers

droplet’s picture

Status: Needs review » Needs work

Please also check the code and do manual tests during reroll.

HOG’s picture

Edited last path, as it not worked correctly. Added styles for filter sort table header for pretty display. Patch work ok now as i see, but i find new error after add new group:
Uncaught TypeError: Cannot read property 'width' of undefined. In function Drupal.dialog~resetSize

droplet’s picture

Status: Needs review » Needs work

We shouldn't do multiple changes in one issue. Please open an issue for CSS part.

i find new error after add new group:
Uncaught TypeError: Cannot read property 'width' of undefined. In function Drupal.dialog~resetSize

is it caused by this patch ?

kostyashupenko’s picture

Issue tags: -Needs reroll
HOG’s picture

@droplet, js bug is in 8.0.x branch also.
About multiple changes i separate it now.

HOG’s picture

Status: Needs work » Needs review
FileSize
539 bytes
818 bytes
zeeshan_khan’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me ;)
Thanks @HOG

HOG’s picture

HOG’s picture

How to test:

  1. Open link: admin/structure/views/view/content
  2. On filter group click on arow (after Add lunk)
  3. Select 'Add or reange'
  4. You see popup: 'Page: Rearrange filter criteria'

  • catch committed ff06c79 on 8.1.x
    Issue #2302515 by HOG, Elijah Lynn, kostyashupenko, camoa, eleleka:...
catch’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x, thanks!

This is just clean-up so not eligible for 8.0.x.

Status: Fixed » Closed (fixed)

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