primefaces / primereact-sass-theme

PrimeReact Theming with SASS
MIT License
35 stars 41 forks source link

Warnings and Code smells #73

Open Ocean15 opened 4 months ago

Ocean15 commented 4 months ago

Hello there!

I get several deprecation warnings from sass with your theme-base:

Deprecation Warning: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls

    ┌──> src\assets\theme-base\components\data\_carousel.scss
6   │               margin: $inlineSpacing;
    │               ^^^^^^^^^^^^^^^^^^^^^^ declaration
    ╵
    ┌──> src\assets\theme-base\_mixins.scss
226 │ ┌     &:focus-visible {
227 │ │         @include focused();
228 │ │     }
    │ └─── nested rule
    ╵
    src\assets\theme-base\components\data\_carousel.scss 6:13  @import
    src\assets\theme-base\_components.scss 42:13               @import
    src\assets\themes\indigoy\theme.scss 3:9                   root stylesheet

Deprecation Warning: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls

    ┌──> src\assets\theme-base\components\data\_treetable.scss
254 │                       margin-right: $inlineSpacing;
    │                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ declaration
    ╵
    ┌──> src\assets\theme-base\_mixins.scss
226 │ ┌     &:focus-visible {
227 │ │         @include focused();
228 │ │     }
    │ └─── nested rule
    ╵
    src\assets\theme-base\components\data\_treetable.scss 254:21  @import
    src\assets\theme-base\_components.scss 52:13                  @import
    src\assets\themes\indigoy\theme.scss 3:9                      root stylesheet

Deprecation Warning: Sass's behavior for declarations that appear after nested
rules will be changing to match the behavior specified by CSS in an upcoming
version. To keep the existing behavior, move the declaration above the nested
rule. To opt into the new behavior, wrap the declaration in `& {}`.

More info: https://sass-lang.com/d/mixed-decls

    ┌──> src\assets\theme-base\components\overlay\_dialog.scss
21  │               margin-right: $inlineSpacing;
    │               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ declaration
    ╵
    ┌──> src\assets\theme-base\_mixins.scss
226 │ ┌     &:focus-visible {
227 │ │         @include focused();
228 │ │     }
    │ └─── nested rule
    ╵
    src\assets\theme-base\components\overlay\_dialog.scss 21:13  @import
    src\assets\theme-base\_components.scss 69:13                 @import
    src\assets\themes\indigoy\theme.scss 3:9                     root stylesheet

I also get a lot of "Unexpected duplicate selector" warnings from my sonarqube because of the theme-base. I can ignore the folder from being scanned but there are some very strange code fragments in your library you should check. For example: /theme-base/components/input/_checkbox.scss line 40-60 and 62-82 contain the exact same code.

Greetings Ocean

HarshThink commented 3 months ago

Solution:

Apply the following changes to resolve the warnings in the dialog, carousel, treetable, and any other components where the warning appears.

Before:

.p-dialog-header-icon {
  @include action-icon();
  margin-right: $inlineSpacing;

  &:last-child {
    margin-right: 0;
  }
}

After:

.p-dialog-header-icon {
  @include action-icon();
  & {
    margin-right: $inlineSpacing;
  }

  &:last-child {
    margin-right: 0;
  }
}

This change should be applied to the dialog component in the file public/assets/theme-base/components/overlay/_dialog.scss. Additionally, apply this solution to the carousel, treetable, and any other components where you encounter similar warnings.

melloware commented 3 months ago

@HarshThink please submit a PR!