php-code-coverage icon indicating copy to clipboard operation
php-code-coverage copied to clipboard

Bootstrap 5 upgrade without theme feature.

Open gammamatrix opened this issue 1 year ago • 8 comments

Changes

  • The theme feature was removed in this branch.
  • See GH-1036

gammamatrix avatar May 11 '24 23:05 gammamatrix

Are these visual changes intentional?

Before these changes (cafa435d750d52e5e7c9c671341483d12d260c45)

grafik

After these changes (ba3047f872113a15df70f8e5d246b0524681f98c)

grafik

sebastianbergmann avatar May 12 '24 05:05 sebastianbergmann

Are these visual changes intentional?

No. I am looking into this now. It could be browser specific.

I am reviewing the rules in style.css; the old rules definitely need a bit of modification due to a new Bootstrap 5 rule for tables:

.table>:not(caption)>*>* {
    padding: .5rem .5rem;
    color: var(--bs-table-color-state,var(--bs-table-color-type,var(--bs-table-color)));
    background-color: var(--bs-table-bg);
    border-bottom-width: var(--bs-border-width);
    box-shadow: inset 0 0 0 9999px var(--bs-table-bg-state,var(--bs-table-bg-type,var(--bs-table-accent-bg)))
}
  • This rule changes the td background-color.
  • I will test a few more browsers and operating systems to make sure the colors propagate from the table.tr to table.tr.td, as expected.
  • I have at least one minor change so far. I think I may have to alter a few more rules, once I test a bit.

gammamatrix avatar May 12 '24 20:05 gammamatrix

Expected output

I updated a few rules, and undid a few modified rules.

  • I have not tested other browsers yet, it is possible there is an issue with the CSS function for rgb(from ...):
rgb(from var(--bs-warning) r g b / 0.1)

This is what I am seeing with the latest changes: php-code-coverage-bootstrap-5-GH-1037

gammamatrix avatar May 12 '24 21:05 gammamatrix

Findings

It does appear to be a browser compatibility issue with only Firefox for relative colors: https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/rgb#browser_compatibility

I went ahead and put back the original default colors into Colors.php:

return new self('#dff0d8', '#c3e3b5', '#99cb84', '#fcf8e3', '#f2dede');

The theme friendly rules can still be applied as environment variables:

<html outputDirectory="output/html" lowUpperBound="50" highLowerBound="90"
    colorSuccessLow="rgb(from var(--bs-purple) r g b / 0.75)"
    colorSuccessMedium="rgb(from var(--bs-blue) r g b / 0.55)"
    colorSuccessHigh="rgb(from var(--bs-teal) r g b / 0.25)"
    colorWarning="rgb(from var(--bs-warning) r g b / 0.25)"
    colorDanger="rgb(from var(--bs-danger) r g b / 0.25)" />

NOTE: Mozilla has approved relative colors, but it has not made it into Firefox yet:

  • https://github.com/mozilla/standards-positions/issues/841

gammamatrix avatar May 12 '24 22:05 gammamatrix

Thank you for working on this. Unfortunately, I found more visual differences:

Are these visual changes intentional?

Before these changes (cafa435d750d52e5e7c9c671341483d12d260c45)

grafik

After these changes (3ab35d131e572e50856f37d055db10de34673c4a)

grafik

sebastianbergmann avatar May 13 '24 05:05 sebastianbergmann

Thank you for working on this. Unfortunately, I found more visual differences:

Are these visual changes intentional?

No problem, happy to help! And no, those changes are not intentional.

In this case, the Bootstrap 5 rules are taking precedence with the use of: bg-success, bg-warning, bg-danger

.bg-success {
  --bs-bg-opacity: 1;
  background-color: rgba(var(--bs-success-rgb),var(--bs-bg-opacity)) !important;
}
  • I will take a look and see if there are some simple CSS modifications I can make to style.css.

php-code-coverage-bootstrap-progress-bars

gammamatrix avatar May 13 '24 19:05 gammamatrix

Findings for progress-bar background colors

So it looks like the optional colors have never overridden the progress-bar. Bootstrap 4 just used a lighter color for bg-success:

.bg-success {
    background-color: #28a745 !important;
}

php-code-coverage-bootstrap-4-used-different-green

I was experimenting with some rules to allow the progress bars to override colors.

This is what it would look like with Bootstrap 4 and the override colors, which is not ideal:

php-code-coverage-bootstrap-4-override-progress-bar-colors

I have a few more things I can try, but a final solution might require the usage of themes along with CSS rgba(). If it is too complicated or messy, I will update this comment with details so we can figure out how to proceed.

gammamatrix avatar May 13 '24 20:05 gammamatrix

Proposed changes (without data-bs-theme feature)

Sorry for so much info in this post

  • We could back out these new --phpunit-*- in this last commit, but it seems the best and cleanest way to get the progress bar colors to match the original green: --phpunit-success-bar: #28a745;
  • At the time of finishing this post, besides the failing unit test, I do not know of any other issues, in the browser or the code. Please let me know if any more are found.

In this last pass, I added root variables for --phpunit at the top of the style.css file. The variables are replaced once and defined at the top of the file:

/* :root {
 --phpunit-success-bar: #28a745;
 --phpunit-success-high: #99cb84;
 --phpunit-success-medium: #c3e3b5;
 --phpunit-success-low: #dff0d8;
 --phpunit-warning: #fcf8e3;
 --phpunit-warning-bar: #ffc107;
 --phpunit-danger: #f2dede;
 --phpunit-danger-bar: #dc3545;
} */

:root {
 --phpunit-success-bar: #28a745;
 --phpunit-success-high: {{success-high}};
 --phpunit-success-medium: {{success-medium}};
 --phpunit-success-low: {{success-low}};
 --phpunit-warning: {{warning}};
 --phpunit-warning-bar: #ffc107;
 --phpunit-danger: {{danger}};
 --phpunit-danger-bar: #dc3545;
}
  • the existing color options get set as before, with no changes to Colors.php. I added a few --phpunit-*-bar rules to cover the old Bootstrap 4 color so it looks the same.
  • So far, I have tested overriding the defined --phpunit-* in the customCssFile and that seems to be working properly in Firefox as well for the dark mode theme.

What is different in style.css

1. Lists and columns
.table tbody tr.covered-by-large-tests, .table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, tr.success td, td.success, li.success, span.success {
 background-color: var(--phpunit-success-low);
}

.table tbody tr.covered-by-medium-tests, .table tbody tr.covered-by-medium-tests td, li.covered-by-medium-tests {
 background-color: var(--phpunit-success-medium);
}

.table tbody tr.covered-by-small-tests, .table tbody tr.covered-by-small-tests td, li.covered-by-small-tests {
 background-color: var(--phpunit-success-high);
}

.table tbody tr.warning, .table tbody tr.warning td, .table tbody td.warning, li.warning, span.warning {
 background-color: var(--phpunit-warning);
}

.table tbody tr.danger, .table tbody tr.danger td, .table tbody td.danger, li.danger, span.danger {
 background-color: var(--phpunit-danger);
}
2. Covered, Not Covered and Not Coverable
.covered-by-small-tests {
 background-color: var(--phpunit-success-high);
}

.covered-by-medium-tests {
 background-color: var(--phpunit-success-medium);
}

.covered-by-large-tests {
 background-color: var(--phpunit-success-low);
}

.not-covered {
 background-color: var(--phpunit-danger);
}

.not-coverable {
 background-color: var(--phpunit-warning);
}
3. Progress Bars

This is the part the required the fix

Typically, we try to avoid using !important; however, Bootstrap 5 uses it for progress bars, so we must as well in order to customize them.

.progress-bar.bg-success {
 background-color: var(--phpunit-success-bar) !important;
}

.progress-bar.bg-warning {
 background-color: var(--phpunit-warning-bar) !important;
}

.progress-bar.bg-danger {
 background-color: var(--phpunit-danger-bar) !important;
}

How to override the rules in the customCssFile:

These rgba() calls do not use the "from" support missing in Firefox:

:root {
 --phpunit-success-bar: rgba(var(--bs-success-rgb), 1);
 --phpunit-success-high: rgba(var(--bs-success-rgb), 0.67);
 --phpunit-success-medium: rgba(var(--bs-success-rgb), 0.33);
 --phpunit-success-low: rgba(var(--bs-success-rgb), 0.1);
 --phpunit-warning: rgba(var(--bs-warning-rgb), 0.1);
 --phpunit-warning-bar: rgba(var(--bs-warning-rgb), 1);
 --phpunit-danger: rgba(var(--bs-danger-rgb), 0.1);
 --phpunit-danger-bar: rgba(var(--bs-danger-rgb), 1);
}
  • The screenshots in this post are using these rules in a file defined in customCssFile to show that it is easy to override and use the new CSS variables.
  • These rules reflect the Bootstrap 5 colors and work with light, dark and custom themes: https://github.com/sebastianbergmann/phpunit/pull/5833

How it looks in the browser

This section shows the way the UI works with the commit:

  • Bootstrap 5: implemented custom CSS variables: --phpunit-* https://github.com/sebastianbergmann/php-code-coverage/pull/1037/commits/05f9d9958b46bd4d0f6a47ae708ac983d8c0948c

The examples showing dark mode were manually edited in the browser:

<html lang="en" data-bs-theme="dark">

Expand the four screenshots:

1. Firefox with CSS Vars

This is the latest changes in style.css. A few more CSS rules were added it the end to make the progress bars match.

phpunit-css-vars-firefox

2. Firefox with Dark Theme enabled

This is here just to show that the variables need to be overridden to support themes.

phpunit-css-vars-firefox-dark-theme-original-colors

3. Firefox with Light Theme enabled

Using rules shown above in customCssFile.

phpunit-css-vars-firefox-light-theme-updated-colors

4. Firefox with Dark Theme enabled

Using rules shown above in customCssFile.

phpunit-css-vars-firefox-dark-theme-updated-colors

Tests

  • NOTE: There is a unit test that fails saying the files do not match:
There was 1 failure:

SebastianBergmann\CodeCoverage\Report\Html\EndToEndTest::testPathCoverageForBankAccountTest
BankAccount.php_path.html not match
Failed asserting that string matches format description.
--- Expected
+++ Actual

/Users/jeremy/srv/sites/site-playground-make/vendor/gammamatrix/playground-make-controller/vendor/phpunit/php-code-coverage/tests/tests/Report/Html/EndToEndTest.php:108
/Users/jeremy/srv/sites/site-playground-make/vendor/gammamatrix/playground-make-controller/vendor/phpunit/php-code-coverage/tests/tests/Report/Html/EndToEndTest.php:56

gammamatrix avatar May 13 '24 22:05 gammamatrix

I just tested with the current state of the update-to-bootstrap-5-without-theme-feature branch (at ea94640a1a7dd6a8cd9eaa5e752a74e3af57ef71) and I still see the large amount of whitespace on each line between the line number and the code that I posted a screenshot of in https://github.com/sebastianbergmann/php-code-coverage/pull/1037#issuecomment-2106675841.

Sorry for not getting back to this sooner.

sebastianbergmann avatar Jul 25 '24 11:07 sebastianbergmann

I just tested with the current state of the update-to-bootstrap-5-without-theme-feature branch (at ea94640) and I still see the large amount of whitespace on each line between the line number and the code that I posted a screenshot of in #1037 (comment).

Sorry for not getting back to this sooner.

No worries, I will update my local branches and have a look at the tests to see why the extra whitespace is appearing in the test.

gammamatrix avatar Jul 26 '24 07:07 gammamatrix

My bad: I just noticed that the large amount of whitespace between the left "edge" and the line number also exists in the current releases and therefore cannot be caused by this PR.

sebastianbergmann avatar Aug 09 '24 08:08 sebastianbergmann

Here is what I currently see:

main (at e73593b1b88df891ed17cee70b13b97bdcadc275)

Directory

grafik

File

grafik

update-to-bootstrap-5-without-theme-feature (at ea94640a1a7dd6a8cd9eaa5e752a74e3af57ef71)

Directory

grafik

File

grafik

Can we restore the styling of the top navigation / breadcrumbs and reduce the whitespace between the line numbers and the beginning of the lines? Thanks!

sebastianbergmann avatar Aug 09 '24 08:08 sebastianbergmann

Breadcrumbs Changes

  • I added another css variable for the breadcrumbs:
:root {
 --phpunit-breadcrumbs: #e9ecef;

These style changes are visible in the inspector on the screenshot:

nav .breadcrumb {
  border-radius: var(--bs-border-radius);
  background-color: var(--phpunit-breadcrumbs);
  padding: .75rem 1rem;
}
  • From Chrome, looked similar on Firefox.

php-code-coverage-GH-1037-breadcrumbs-restored-ff

  • Firefox

php-code-coverage-GH-1037-breadcrumbs-restored-chrome

  • Chrome

Line number changes

The line number column had a text-right CSS class that I switched to text-end.

CSS Variables

The template variables in style.css

:root {
 --phpunit-breadcrumbs: var(--bs-gray-200);
 --phpunit-success-bar: #28a745;
 --phpunit-success-high: {{success-high}};
 --phpunit-success-medium: {{success-medium}};
 --phpunit-success-low: {{success-low}};
 --phpunit-warning: {{warning}};
 --phpunit-warning-bar: #ffc107;
 --phpunit-danger: {{danger}};
 --phpunit-danger-bar: #dc3545;
}

Bootstrap 4 was using Gray 200. I updated the breadcrumbs to use the variable.

php-code-coverage-GH-1037-breadcrumbs-phpunit-css-vars

gammamatrix avatar Aug 10 '24 06:08 gammamatrix

Sorry for bothering you with this, but after merging this I see the whitespace left of the line numbers again:

grafik

Also, I think the line numbers were not underlined in the past.

sebastianbergmann avatar Sep 05 '24 14:09 sebastianbergmann

Sorry for bothering you with this, but after merging this I see the whitespace left of the line numbers again:

grafik

Also, I think the line numbers were not underlined in the past.

I will have a look.

gammamatrix avatar Sep 05 '24 18:09 gammamatrix

Dev Notes

I was able to get a rule that works:

table#code {
  td:nth-child(1) {
    padding-left: .75em;
    padding-right: .75em;
    a {
      text-decoration: none;
     }
  }
}
  • Bootstrap 5 no longer adds text-decoration: none; to links.

GH-1037-css-fix-for-first-column

I will open up a new PR instead of using this branch.

gammamatrix avatar Sep 05 '24 18:09 gammamatrix

Changes

  • See: https://github.com/sebastianbergmann/php-code-coverage/pull/1046

gammamatrix avatar Sep 05 '24 18:09 gammamatrix