Add checks for ESlint and fix ESlint issues #488

Neslee · 2024-02-27T10:05:33Z

Linked issues

Fixes Update dependencies based on dxpr_builder repo #486
Fixes Fix ESLint issues #485

Solution

Add check, and fix 200+ failures

Checklist

Neslee · 2024-02-27T10:06:43Z

@jjroelofs I have created this new PR on top of your PR #487, so i can request your review once it's ready.

jjroelofs · 2024-02-27T15:58:20Z

/qa-demo-2x-bs5-tests

Neslee · 2024-03-05T12:53:23Z

/qa-demo-2x-bs5-tests

jjroelofs · 2024-03-05T16:58:27Z

@Neslee "48 errors and 0 warnings potentially fixable with the --fix option." did you try the autofix option?

Neslee · 2024-03-05T16:59:48Z

@jjroelofs yes, those are fixed in my local, will push in next commit.

Neslee · 2024-03-05T17:17:33Z

/qa-demo-2x-bs5-tests

jjroelofs · 2024-03-05T18:49:28Z

/qa-demo-2x-bs5-tests

jjroelofs · 2024-03-05T21:26:20Z

Exception: Tue Mar 05 19:06:00 UTC 2024 SEVERE http://nginx/core/assets/vendor/jquery/jquery.min.js?v=3.6.0 1:31702 Uncaught TypeError: Cannot read properties of null (reading 'classList')
java.lang.Exception: Tue Mar 05 19:06:00 UTC 2024 SEVERE http://nginx/core/assets/vendor/jquery/jquery.min.js?v=3.6.0 1:31702 Uncaught TypeError: Cannot read properties of null (reading 'classList')

@Neslee the QA tests are now expect to completely pass and there are just 10 errors remaining in the editor test suite, the above is one of those that should be fixable. I'm not sure what is up with the missing images, I think I need to still update some code from the test workflow files or something.

Neslee · 2024-03-06T07:37:18Z

/qa-demo-2x-bs5-tests

Neslee · 2024-03-06T09:22:58Z

/qa-demo-2x-bs5-tests

Neslee · 2024-03-06T10:05:54Z

/qa-demo-2x-bs5-tests

https://github.com/dxpr/dxpr_theme/actions/runs/8170262836/job/22337373159

Neslee · 2024-03-06T10:36:18Z

@jjroelofs All errors have been fixed from editor tests, but there are some with timeout and expected true but false

jjroelofs · 2024-03-07T09:29:17Z

If I run tests on the 6.x branch I get these failures:

***_builder_project_maven | [ERROR]   ImageCarouselIntervalOptionVisualTest.verifyTheIntervalOption:60 expected [false] but found [true]
***_builder_project_maven | [ERROR]   JumbotronAddElementVisualTest.verifyTheUserCanAddElementInsideJumbotronElement:47 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsAddTabOptionVisualTest.verifyAddTabOption:41 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsDirectionOptionVisualTest.verifyThatUserCanChangeDirectionOfTabs:52 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsFocusAfterChangeTitleVisualTest.verifyThatUserCanAfterChangeTitleForSecondTabFocusBeInTheSecondTab:55 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsMobileFriendlyOptionVisualTest.verifyThatUserCanChangeMobileFriendlyOption:71 expected [false] but found [true]
***_builder_project_maven | [ERROR]   ToggleEditorVisualTest.verifyToggleEditor:42
***_builder_project_maven | [ERROR]   VimeoVideoWithImageAndThemedPlayedButtonVisualTest>ChromeTestBase.afterClass:242 »

On your branch I get these:

***_builder_project_maven | [ERROR]   JumbotronAddElementVisualTest.verifyTheUserCanAddElementInsideJumbotronElement:47 expected [false] but found [true]
***_builder_project_maven | [ERROR]   ModelCK5AddImageVisualTest.insertAndVerifyCK5EditorImageLinkOption:41 expected [false] but found [true]
***_builder_project_maven | [ERROR]   ModelCK5ImageAlignmentVisualTest.verifyCK5EditorImageAlignmentOption:44 » Timeout
***_builder_project_maven | [ERROR]   ModelCK5ImageAlignmentVisualTest.verifyCK5EditorImageAlignmentOption:44 » Timeout
***_builder_project_maven | [ERROR]   ModelCK5ImageAlignmentVisualTest.verifyCK5EditorImageAlignmentOption:44 » Timeout
***_builder_project_maven | [ERROR]   ModelCK5ImageCaptionVisualTest.addAndVerifyCK5EditorImageCaptionOption:42 » Timeout
***_builder_project_maven | [ERROR]   ModelCK5InlineImageOptionVisualTest.insertAndVerifyCK5EditorImageLinkOption:42 expected [false] but found [true]
***_builder_project_maven | [ERROR]   ModelCK5ResizeImageVisualTest.verifyCK5EditorImageResizeOption:44 » Timeout Ex...
***_builder_project_maven | [ERROR]   ModelCK5ResizeImageVisualTest.verifyCK5EditorImageResizeOption:44 » Timeout Ex...
***_builder_project_maven | [ERROR]   ModelCK5SideImageOptionVisualTest.sideCK5EditorImageAndVerify:45 expected [false] but found [true]
***_builder_project_maven | [ERROR]   ModelCK5TableMergeOptionsVisualTest.verifyCK5EditorMergeCellTable:73 » MoveTargetOutOfBounds
***_builder_project_maven | [ERROR]   TabsAddTabOptionVisualTest.verifyAddTabOption:41 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsDirectionOptionVisualTest.verifyThatUserCanChangeDirectionOfTabs:52 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsFocusAfterChangeTitleVisualTest.verifyThatUserCanAfterChangeTitleForSecondTabFocusBeInTheSecondTab:55 expected [false] but found [true]
***_builder_project_maven | [ERROR]   TabsMobileFriendlyOptionVisualTest.verifyThatUserCanChangeMobileFriendlyOption:71 expected [false] but found [true]
***_builder_project_maven | [ERROR]   ToggleEditorVisualTest.verifyToggleEditor:42
***_builder_project_maven | [ERROR]   VimeoVideoWithImageAndThemedPlayedButtonVisualTest>ChromeTestBase.afterClass:242 » 
***_builder_project_maven | [ERROR]   YouTubeWithImageAndThemedPlayedButtonVisualTest>ChromeTestBase.afterClass:242 »

I will manually test the additional failures' related features and also run the tests again to see if it is consistent.

jjroelofs · 2024-03-07T09:31:39Z

/qa-demo-2x-bs5-tests

Neslee · 2024-03-08T06:16:04Z

@jjroelofs did you find some time to look into this editor test fails, seems like it's running forever.

Neslee · 2024-03-08T06:19:50Z

@jjroelofs just checked now there are 18 editor test failures in 6.x branch - #491
In this pr changes there are 17 editor test failures, so probably we have merged something to 6.x without running these tests

jjroelofs · 2024-03-08T06:59:03Z

@Neslee I think the problem is also happening in dxpr_builder repo which uses dxpr_theme:5.x. So, this is probably a problem with the tests themselves, they are new tests.

js/dist/dxpr-theme-full-screen-search.js

jjroelofs

dxpr-theme-header.min.js?s9weip:1 Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Element': '> li' is not a valid selector.

I see that 80% of the work is in dxpr-theme-header.js but since the file is now throwing an error on every page I'll let you fix that first before manually testing your changes. If you haven't already please do manual testing yourself of the header's various functions that you worked on.

jjroelofs · 2024-03-08T12:46:11Z

dxpr_theme.post_update.php

  foreach ($theme_list as $theme) {
    $theme_name = $theme->getName();
-    if ('dxpr_theme' === ($theme->info['base theme'] ?? '') OR 'dxpr_theme' === $theme_name) {
+    if ('dxpr_theme' === ($theme->info['base theme'] ?? '') or 'dxpr_theme' === $theme_name) {


I've never seen this in PHP, does it even work? I'm pretty sure a lowercase "or" is not in Drupal coding standards. This PR should also not touch any PHP.

Neslee · 2024-03-10T12:22:59Z

@jjroelofs I tried a lot to squash commits but i am getting errors and git is throwing me various warnings and making conflicts when I do it

jjroelofs

Great, dropdowns are working fine now :)

Jurriaan Roelofs and others added 4 commits February 27, 2024 06:39

update dependencies

e154899

Update config

a2be122

update dependencies

04527d2

Mutlilevel mobile menu and header eslint fixes

1a96f1e

Merge branch '6.x' into neslee/6.x/#485-fix-eslint-errors

cfafd88

Header and full search eslint fixes

aa13fab

More eslint fixes in header and full serach

3306c64

Eslint autofixes and manual fixes to header and search

3782e60

Merge branch '6.x' into neslee/6.x/#485-fix-eslint-errors

c3e4605

Neslee added 5 commits March 6, 2024 10:46

Resolved conflicts

ebebc9d

Eslint autofixes

e47ee74

Drupal lint autofixes

a9736d3

Fixed final round of eslint on header

8bc18a4

Fixed classList editor test error

3e8f437

Header menu eslint editor test fixes

712d9e2

Neslee added 2 commits March 6, 2024 15:29

Header menu eslint editor test fixes

d822c67

Eslint auto fixes

6688855

Neslee requested a review from jjroelofs March 6, 2024 10:36

Neslee self-assigned this Mar 7, 2024

jjroelofs requested changes Mar 8, 2024

View reviewed changes

js/dist/dxpr-theme-full-screen-search.js Show resolved Hide resolved

Neslee changed the title ~~Add check that prohibits jQuery functions and Fix eslint errors~~ Add checks for ESlint and fix ESlint issues Mar 8, 2024

Neslee requested a review from jjroelofs March 8, 2024 07:27

jjroelofs requested changes Mar 8, 2024

View reviewed changes

Fixed dropdown element eslint and functionality

4f748e2

Neslee requested a review from jjroelofs March 10, 2024 12:21

jjroelofs approved these changes Mar 11, 2024

View reviewed changes

jjroelofs merged commit 60fcb12 into 6.x Mar 11, 2024

jjroelofs deleted the neslee/6.x/#485-fix-eslint-errors branch March 11, 2024 11:17

Add checks for ESlint and fix ESlint issues #488

Add checks for ESlint and fix ESlint issues #488

Uh oh!

Conversation

Neslee commented Feb 27, 2024

Linked issues

Solution

Checklist

Uh oh!

Neslee commented Feb 27, 2024

Uh oh!

jjroelofs commented Feb 27, 2024

Uh oh!

Neslee commented Mar 5, 2024

Uh oh!

jjroelofs commented Mar 5, 2024

Uh oh!

Neslee commented Mar 5, 2024

Uh oh!

Neslee commented Mar 5, 2024

Uh oh!

jjroelofs commented Mar 5, 2024

Uh oh!

jjroelofs commented Mar 5, 2024

Uh oh!

Neslee commented Mar 6, 2024

Uh oh!

Neslee commented Mar 6, 2024

Uh oh!

Neslee commented Mar 6, 2024 • edited by jjroelofs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neslee commented Mar 6, 2024

Uh oh!

jjroelofs commented Mar 7, 2024

Uh oh!

jjroelofs commented Mar 7, 2024

Uh oh!

Neslee commented Mar 8, 2024

Uh oh!

Neslee commented Mar 8, 2024

Uh oh!

jjroelofs commented Mar 8, 2024

Uh oh!

Uh oh!

jjroelofs left a comment

Choose a reason for hiding this comment

dxpr-theme-header.min.js?s9weip:1 Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Element': '> li' is not a valid selector.

Uh oh!

jjroelofs Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

Neslee commented Mar 10, 2024

Uh oh!

jjroelofs left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Neslee commented Mar 6, 2024 •

edited by jjroelofs

Loading