Problem/Motivation

In order to create better themability of the Svelte app, it would be nice if our markup would provide BEM-style syntax to make the most lightweight selectors possible for other themes to be able to style the Project Browser.

Proposed resolution

(1) Evaluate class names used to be in BEM syntax.
(2) Update those class names
(3) remove the hashed classes produced and used by Svelte altogether, so we're only using the classes we write, so maximize overrideability by themes

Issue fork project_browser-3365435

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

chrisfromredfin created an issue. See original summary.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Issue tags: +core-mvp
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Issue tags: -Project Browser Initiative +stable blocker

andy-blum made their first commit to this issue’s fork.

andy-blum opened merge request !416

andy-blum’s picture

andy-blum
he/him/his
Primary language English
Location Ohio, USA
commented

Marked as ready to kick off an initial round of tests to make sure I'm on the right track.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

Current fail is due to not running prettier on the codebase before committing, I believe you can just do `yarn prettier` then commit and push.

andy-blum’s picture

andy-blum
he/him/his
Primary language English
Location Ohio, USA
commented

Adding a comment here just to bump this issue back to the top of my dashboard.

bernardm28 made their first commit to this issue’s fork.

bernardm28’s picture

bernardm28 commented

The issue seemed to be for most test errors that we had .project instead of .pb-project which could not be found.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Active » Needs work

bernardm28 opened merge request !437

bernardm28’s picture

bernardm28 commented

The last merge request works on my local, could It be a timeout issue with the functional PHP unit test, on drupal ci?

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

If you compare the DrupalCI test fails and the GitLab CI test fails, are they the same? (Might need to rebase to get DrupalCI to fail. If they're different, then it's just flakiness all around... :( )

bernardm28 opened merge request !441

bernardm28’s picture

bernardm28 commented

They were not the same last time I checked.

ahsannazir made their first commit to this issue’s fork.

ahsannazir’s picture

ahsannazir commented
Status: Needs work » Needs review
bernardm28’s picture

bernardm28 commented

No luck so far but closer atleast.

bernardm28 changed the visibility of the branch 3365435-update-markup-classnames to hidden.

bernardm28 changed the visibility of the branch 3365435-update-markup-classnames-rebased-test to hidden.

mike.roman’s picture

mike.roman commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +MidCamp 2024

I reviewed the module and noticed no styling regressions as a result of all the class name changes.

bernardm28’s picture

bernardm28 commented
Issue tags: +FLDC2024

I spent some time looking at it today, I did not notice any visual regression.
Also, I added the FLDC2024 tag as I started working on this for Florida drupal camp contribution day.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Fixed

Marking this fixed, because there IS a test failure, but it is failing on HEAD also--a result of us chasing HEAD on automatic_updates (package_manager).

And this is a big-bad, and getting it in means we can rebase and move forward on a number of smaller issues. 

There was 1 failure:
    
    1)
    Drupal\Tests\project_browser\FunctionalJavascript\ProjectBrowserInstallerUiTest::testInstallModuleAlreadyInFilesystem
    Failed asserting that a NULL is not empty.
    
    /builds/project/project_browser/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
    /builds/project/project_browser/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:56
    /builds/project/project_browser/tests/src/FunctionalJavascript/ProjectBrowserInstallerUiTest.php:106
    /builds/project/project_browser/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
    
    FAILURES!
    Tests: 5, Assertions: 48, Failures: 1.

Status: Fixed » Closed (fixed)

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