Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uses logical properties for fields.css file #635

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

markconroy
Copy link
Member

Closes #627

What does this change?

Rewrites the fields.css file to use logical properties. By doing this we can cut down a lot on how much CSS we write and send to the browser.

How to test

Only inline field labels are affected by this file, so set a field label to inline and make sure it works the same as currently.


Thanks to Big Blue Door for sponsoring my time to work on this.

@millnut
Copy link
Member

millnut commented Oct 22, 2024

Hey @markconroy this is the error from the test, I wonder whether this branch was created when the helper was in 1.x, and since it's been reverted this branch needs to be rebased?

PHPUnit 9.6.21 by Sebastian Bergmann and contributors.

Testing /app/web/themes/contrib/localgov_base
ER                                                                  2 / 2 (100%)E

Time: 02:33.901, Memory: 10.00 MB

There were 2 errors:

1) Drupal\Tests\block_content\Functional\RegionRenderTest::testRegionRender with data set #0 ('disabled', 'FALSE')
Drupal\Core\Extension\MissingDependencyException: Unable to install theme: 'localgov_base' due to unmet module dependencies: 'localgov_base_helper'.

/app/web/core/lib/Drupal/Core/Extension/ThemeInstaller.php:176
/app/web/core/includes/install.core.inc:1654
/app/web/core/includes/install.core.inc:695
/app/web/core/includes/install.core.inc:572
/app/web/core/includes/install.core.inc:121
/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:315
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:570
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:370
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

2) Drupal\Tests\block_content\Functional\RegionRenderTest::testRegionRender with data set #1 ('sidebar_first', 'TRUE')
Drupal\Core\Extension\MissingDependencyException: Unable to install theme: 'localgov_base' due to unmet module dependencies: 'localgov_base_helper'.

/app/web/core/lib/Drupal/Core/Extension/ThemeInstaller.php:176
/app/web/core/includes/install.core.inc:1654
/app/web/core/includes/install.core.inc:695
/app/web/core/includes/install.core.inc:572
/app/web/core/includes/install.core.inc:121
/app/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:315
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:570
/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:370
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

--

There was 1 risky test:

1) Drupal\Tests\block_content\Functional\RegionRenderTest::testRegionRender with data set #0 ('disabled', 'FALSE')
This test did not perform any assertions

/app/web/core/tests/Drupal/Tests/Listeners/DrupalListener.php:62
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:453
/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
/app/vendor/phpunit/phpunit/src/Framework/TestSuite.php:685
/app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/app/vendor/phpunit/phpunit/src/TextUI/Command.php:146
/app/vendor/phpunit/phpunit/src/TextUI/Command.php:99

ERRORS!
Tests: 2, Assertions: 0, Errors: 2, Risky: 1.

@markconroy
Copy link
Member Author

@millnut All tests passing now. Okay for this to merge?

@millnut
Copy link
Member

millnut commented Oct 24, 2024

@markconroy yep LGTM 👍

@millnut millnut self-requested a review October 24, 2024 06:51
@markconroy markconroy merged commit 1fe3800 into 1.x Oct 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moderise the field.css
2 participants