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

bug: missing root element's scope id as classname in nested child #5702

Closed
3 tasks done
yigityuce opened this issue Apr 25, 2024 · 4 comments
Closed
3 tasks done

bug: missing root element's scope id as classname in nested child #5702

yigityuce opened this issue Apr 25, 2024 · 4 comments
Assignees
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@yigityuce
Copy link
Contributor

Prerequisites

Stencil Version

4.17.1

Current Behavior

think about having a component structure like below:

my-timepicker -------> my-input ---------> input (native)
             |
             | ------> my-menu --------> my-popover ---------> whatever


my-popover (can be used as standalone)

given that component structure I have a style for popover (eg.: padding: 8;) and I want to customize popover style from the timepicker component's style file like:

:host {
  my-menu {
     my-popover {
        padding: 32px;
     }
  }
}

Current behavior: nested element selector is NOT working since during the scss compilation phase it adds .sc-my-timepicker class to the my-popover selector and the compiled CSS file becomes sth like this:

.sc-my-timepicker-h my-menu.sc-my-timepicker my-popover.sc-my-timepicker {
  padding: 32px;
}

the problem is root component's scope id (sc-my-timepicker) is NOT being added to the nested component's classNames. So the selector which is defined in root component is NOT working.

Expected Behavior

works as expected

current:

current

expected:

expected

System Info

Your current version of Node is v16.18.0, however Stencil's recommendation is v18.16.0 or greater. Note that future versions of Stencil will eventually remove support for older Node versions and an Active LTS version is recommended (https://nodejs.org/en/about/releases/).


      System: node 16.18.0
    Platform: darwin (23.4.0)
   CPU Model: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz (12 cpus)
    Compiler: /Users/yigit.yuce/Documents/Projects/personal/yigityuce.github/stencil-v4-sc-nested-css-scope-id/node_modules/@stencil/core/compiler/stencil.js
       Build: 1713902307
     Stencil: 4.17.1 🚒
  TypeScript: 5.4.5
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.30.3

Steps to Reproduce

https://github.com/yigityuce/stencil-v4-sc-nested-css-scope-id

Code Reproduction URL

https://github.com/yigityuce/stencil-v4-sc-nested-css-scope-id

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Apr 25, 2024
yigityuce added a commit to yigityuce/stencil that referenced this issue Apr 25, 2024
sass compiler adds root scope id to the nested child elements too, therefore nested element selectors are not working from the root component since the root scope id is not adding as classname to the nested children elements

ionic-team#5702
@christian-bromann christian-bromann self-assigned this Apr 25, 2024
@christian-bromann
Copy link
Member

Thanks for raising this well described bug and providing a reproducible example. I took a look and reviewed the PR.

@yigityuce
Copy link
Contributor Author

@christian-bromann PR is updated

github-merge-queue bot pushed a commit that referenced this issue May 3, 2024
* fix(runtime): add root scope id to the nested child as classname

sass compiler adds root scope id to the nested child elements too, therefore nested element selectors are not working from the root component since the root scope id is not adding as classname to the nested children elements

#5702

* test: add additional test for css and add code documentation

* test: fix failing unit test due to the additional class

* chore: remove shadowDom condition regarding review
@rwaskiewicz rwaskiewicz added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels May 3, 2024
@yigityuce
Copy link
Contributor Author

closing the issue since #5704 is merged 🚀 thanks all!

@rwaskiewicz
Copy link
Contributor

The fix for this PR was included in today's v4.18.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

No branches or pull requests

3 participants