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

Warnings and Code smells #73

Open
Ocean15 opened this issue Jul 23, 2024 · 2 comments
Open

Warnings and Code smells #73

Ocean15 opened this issue Jul 23, 2024 · 2 comments

Comments

@Ocean15
Copy link

Ocean15 commented Jul 23, 2024

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
Copy link

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
Copy link
Member

@HarshThink please submit a PR!

voringer added a commit to voringer/primereact-sass-theme that referenced this issue Aug 14, 2024
 Warnings and Code smells primefaces#73
voringer added a commit to voringer/primereact-sass-theme that referenced this issue Aug 14, 2024
 Warnings and Code smells primefaces#73
voringer added a commit to voringer/primereact-sass-theme that referenced this issue Aug 14, 2024
 Warnings and Code smells primefaces#73
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

No branches or pull requests

3 participants