Project list should be using CSS grid and use gaps instead of margins

CI is returning the following error:

Svelte compiled files do not match. Running yarn build and commiting the changs should fix this.

Can you re-run yarn build and push the changes?

Thanks for catching @mherchel! Still learning where to look for build errors like that.

Still learning where to look for build errors like that.

Yeah, D.O doesn't make it easy.

If you have a "custom commands failed", it could mean either nightwatch tests fail, or cspell, or quality checks (where the output isn't as expected). In this case, I clicked on the link for "custom commands failed", which was https://www.drupal.org/pift-ci-job/2722419 → . I typically hit CMD+F and search for "fail".

If you scroll down, you'll see "Svelte compiled files do not match. Running yarn build and commiting the changs should fix this."

Troubleshooting nightwatch errors are more difficult, but that's another situation.

Anyway, now it looks like we're getting some legit failures within the functional JavaScript tests.

You can find that by clicking on the most recent failures. You'll see that it says

There was 1 failure:

1) Drupal\Tests\project_browser\FunctionalJavascript\ProjectBrowserUiTestJsonApi::testGrid

So, search the codebase for testGrid and see what its trying to do. Fix the selectors or whatever and see if you can get it working.

Functional JS tests are a pain in the ass IMO, so if you can't get it working, let me know and we can bring in some reinforcements.

Thanks for working on this!

So, search the codebase for testGrid and see what its trying to do. Fix the selectors or whatever and see if you can get it working.

Got it, thanks @mhershel!

I found where the tests are, but I'd like to be able to run tests locally to reduce cycles to manageable levels while I try to fix the issue. Unfortunately, I can't seem to get them working. I'm running the below:

ddev php web/core/scripts/run-tests.sh --url https://drupal-project-browser.ddev.site --color --directory modules/contrib/project_browser --dburl mysql://root@127.0.0.1/db

And get this error:

PHP Fatal error:  Uncaught ValueError: Path cannot be empty in /var/www/html/web/core/tests/Drupal/TestTools
/PhpUnitCompatibility/ClassWriter.php:58
Stack trace:
#0 /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/ClassWriter.php(58): file_get_contents
('')
#1 /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/ClassWriter.php(34): Drupal\TestTools\
PhpUnitCompatibility\ClassWriter::alterAssert(Object(Composer\Autoload\ClassLoader))
#2 /var/www/html/web/core/scripts/run-tests.sh(504): Drupal\TestTools\PhpUnitCompatibility\ClassWriter::muta
teTestBase(Object(Composer\Autoload\ClassLoader))
#3 /var/www/html/web/core/scripts/run-tests.sh(57): simpletest_script_init()
#4 {main}
  thrown in /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/ClassWriter.php on line 58

Fatal error: Uncaught ValueError: Path cannot be empty in /var/www/html/web/core/tests/Drupal/TestTools/PhpU
nitCompatibility/ClassWriter.php:58
Stack trace:
#0 /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/ClassWriter.php(58): file_get_contents
('')
#1 /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/ClassWriter.php(34): Drupal\TestTools\
PhpUnitCompatibility\ClassWriter::alterAssert(Object(Composer\Autoload\ClassLoader))
#2 /var/www/html/web/core/scripts/run-tests.sh(504): Drupal\TestTools\PhpUnitCompatibility\ClassWriter::muta
teTestBase(Object(Composer\Autoload\ClassLoader))
#3 /var/www/html/web/core/scripts/run-tests.sh(57): simpletest_script_init()
#4 {main}
  thrown in /var/www/html/web/core/tests/Drupal/TestTools/PhpUnitCompatibility/ClassWriter.php on line 58
Failed to run php web/core/scripts/run-tests.sh --url https://drupal-project-browser.ddev.site --color --dir
ectory modules/contrib/project_browser --dburl mysql://root@127.0.0.1/db: exit status 255

Spent time experimenting with different commands, and looking at the command that the Drupal tests run themselves, but so far no luck in getting it to work. Do you have any insight here? If not, I can just elongate my cycles and push known bad code to d.o and see what the tests do.

We are moving in the wrong direction.

I ran these tests locally and was unable to replicate the errors associated with #5e6988c5. I wanted to re-run the tests, but I don't see that as an option anymore on the d.o test results page.

I've updated a CSS selector for the sake of making a new push instead.

Interestingly, when I run locally, I get these 3 fails.

It seems like there are more changes happening in the Svelte files than needed, particularly the changed breakpoints. I'm also quite open to the possibility that this is a byproduct of me not fully getting it yet 🙂.

There's definitely a need to have the enclosing <ul> be aware of the grid/list view, which on HEAD is only available to the <li> elements. It looks like that is part of what the MR is doing. I was able to get that taken care of with fewer overall changes to the .svelte files. It may be best to centralize the toggleView available to <ProjectGrid> and <Project> by adding it to the store, but what I have below seemed to work really well with the CSS changes from the MR

diff --git a/sveltejs/src/ProjectBrowser.svelte b/sveltejs/src/ProjectBrowser.svelte
index b8a6d42e..4a232d2b 100644
--- a/sveltejs/src/ProjectBrowser.svelte
+++ b/sveltejs/src/ProjectBrowser.svelte
@@ -293,7 +293,7 @@
 </script>
 
 <MediaQuery query="(min-width: 1200px)" let:matches>
-  <ProjectGrid {loading} {rows} {pageIndex} {$pageSize} let:rows>
+  <ProjectGrid toggleView={!matches ? 'Grid' : toggleView} {loading} {rows} {pageIndex} {$pageSize} let:rows>
     <div slot="head">
       <Tabs {dataArray} on:tabChange={toggleRows} />
       <Search
diff --git a/sveltejs/src/ProjectGrid.svelte b/sveltejs/src/ProjectGrid.svelte
index 2ee0de85..14c57d74 100644
--- a/sveltejs/src/ProjectGrid.svelte
+++ b/sveltejs/src/ProjectGrid.svelte
@@ -21,6 +21,8 @@
     loading: Drupal.t('Loading data'),
   };
 
+  export let toggleView;
+
   $: filteredRows = rows;
   $: visibleRows = filteredRows
     ? filteredRows.slice(pageIndex, pageIndex + $pageSize)
@@ -58,7 +60,7 @@
         <div>{@html labels.empty}</div>
       {:else}
         <ul
-          class="projects-list"
+          class="projects-{toggleView.toLowerCase()}"
           id={$activeTab}
           role="tabpanel"
           tabindex="0"

The changes to <MediaQuery> can be seen in isolation here

The goal here is to avoid needing to add in duplicate MediaQuery components to get effects at the same breakpoint. The MQ component now adds that value into a Map where the key is the exact media query string and the value is a boolean.

With this logic, we can now update nested components to simply reference and respond to the values in the store like in this commit

The exact reason this was needed in the first place is that the project grid/list toggles back and forth between the two for both user intiated reasons, as well as automatically when the browser is resized. Prior to this change the following could happen:

• User loads page on desktop
• User toggles to list view
• User resizes browser down to "mobile" breakpoint
• User resizes browser up to "desktop" breakpoint and the layout is automatically switched back to grid

Now, the value of toggleView is always directly linked to the user's specified preference and we're able to override that when the viewport width necessitates it.

In a highly-simplified version of the code:

ProjectBrowser.svelte handles the media query as well as the user preferences from the list/grid buttons.

<MediaQuery query="(min-width: 1200px)" let:matches>
  <ProjectGrid {toggleView} {loading} {rows} {pageIndex} {$pageSize} let:rows>
    <button on:click={}>List</button>
    <button on:click={}>Grid</button>

    {#each rows as row, index (row)}
      <Project {toggleView} project={row} />
    {/each}
    
  </ProjectGrid>
</MediaQuery>

ProjectGrid.svelte adds the preferred class on desktop breakpoints and the list class on smaller widths

<ul class="projects-{isDesktop ? toggleView.toLowerCase() : 'list'}">
  <slot rows={visibleRows} />
</ul>

Project.svelte does the same thing as ProjectGrid.svelte

<li class="project project--{isDesktop ? toggleView.toLowerCase() : 'list'}">
  
</li>

Linking discussion thread here:
https://drupal.slack.com/archives/C01UHB4QG12/p1698244456535349

Summary is that we think the current state of passing tests is correct, in that we did change the breakpoints for grid vs. list so we need to update the svelteInitHelper to look for a different one.

I would like Andy to confirm that we did change this intentionally around when it goes to grid mode, but if so, I think we can RTBC this.

Confirmed!

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