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.

Sign up for GitHub

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

Jump to bottom

Content added to empty scrim is not rendered #7032

Closed
nwhittaker opened this issue May 25, 2023 · 7 comments
Closed

Content added to empty scrim is not rendered #7032

nwhittaker opened this issue May 25, 2023 · 7 comments
Assignees
@geospatialem
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 3 A day or two of work, likely requires updates to tests. has workaround Issues have a workaround available in the meantime. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - medium Issue is non core or affecting less that 60% of people using the library
Milestone
1.6.0 - Low risk ...

Comments

@nwhittaker
Copy link
Contributor

Actual Behavior

Given an empty scrim that is already rendered, appending text content or child nodes results in no visual change to the scrim.

Screenshot 2023-05-25 at 3 43 55 PM

Expected Behavior

The appended content becomes visible within the scrim.

Screenshot 2023-05-25 at 3 48 10 PM

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/yLRwjZw

Reproduction Steps

  1. Visit the sample and use dev tools to inspect the scrim
  2. Within a second of rendering the scrim, see the DOM is updated with the scrim's new text content, however the text is not rendered in the preview area.

Reproduction Version

1.4.0

Relevant Info

Interestingly, if the scrim is not initially empty, updates to its content are rendered. A workaround is to seed the empty scrim with a comment.

<calcite-scrim><!-- --></calcite-scrim>

Regression?

No response

Priority impact

p4 - not time sensitive

Impact

No response

Esri team

ArcGIS Field Apps
@nwhittaker nwhittaker added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels May 25, 2023
@github-actions github-actions bot added ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive labels May 25, 2023
@jcfranco jcfranco added the has workaround Issues have a workaround available in the meantime. label May 26, 2023
@geospatialem geospatialem added estimate - 3 A day or two of work, likely requires updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed needs triage Planning workflow - pending design/dev review. labels Aug 7, 2023
@geospatialem geospatialem added this to the 2023 August Priorities milestone Aug 7, 2023
@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. labels Aug 7, 2023
driskull added a commit that referenced this issue Aug 7, 2023
@driskull
fix(scrim): handle slotted children correctly. #7032
75f0ef8
@driskull driskull mentioned this issue Aug 7, 2023
Merged
driskull added a commit that referenced this issue Aug 8, 2023
@driskull
fix(scrim): handle slotted children correctly (#7477)
c5ce008 
**Related Issue:** #7032

## Summary

- Support adding slotted content after initial render
- Uses `onSlotChange` to detect slotted elements
- Hides content container via the hidden attribute
- Updates test
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Aug 8, 2023
@github-actions github-actions bot assigned geospatialem and unassigned driskull Aug 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Aug 10, 2023
@geospatialem
Copy link
Member

This looks as-expected in 1.6.0-next.3, but differs from the screenshots above. cc @nwhittaker In case additional verification is needed with the use case above, as it differs from the repro sample.

image

@nwhittaker
Copy link
Contributor Author

@geospatialem, sorry for the bad repro code -- I must have saved over it at some point. I updated the original codepen to match the repro steps.

I can confirm that appended child nodes are now appearing as expected. However, I still don't see text nodes appearing, and the latest appears to regress the workaround with seeding the scrim with a <!-- -->.

Here's the same codepen upgraded to 1.6.0-next.3: https://codepen.io/nwhittaker-esri/pen/NWEVQLO. Repro steps should be the same.

@geospatialem
Copy link
Member

geospatialem commented Aug 10, 2023 •
edited
Loading

Thanks for following up, @nwhittaker!

It does look like this seems to be a regression between 1.2.0 and 1.3.0. The only change I can see between the two releases is the introduction of design tokens and utilities in the comparison. Nevermind, weird intermittent internet made this seem like a regression, it has not been supported in previous releases.

Will follow up with research to determine next steps.

@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. research Issues that require more in-depth research or multiple team members to resolve or make decision. and removed 4 - verified Issues that have been released and confirmed resolved. labels Aug 10, 2023
@geospatialem geospatialem added regression Issues that are caused by changes in a release, but were working before that. and removed regression Issues that are caused by changes in a release, but were working before that. labels Aug 10, 2023
@geospatialem geospatialem added 0 - new New issues that need assignment. and removed 1 - assigned Issues that are assigned to a sprint and a team member. research Issues that require more in-depth research or multiple team members to resolve or make decision. labels Aug 10, 2023
@geospatialem
Copy link
Member

The fix will be added as part of the September release.

There is a workaround when adding a child div element to the scrim: https://codepen.io/geospatialem/pen/wvQLwLB.

driskull added a commit that referenced this issue Aug 11, 2023
@driskull
fix(scrim): render text content inside scrim. #7032
1603908
@driskull driskull mentioned this issue Aug 11, 2023
Merged
driskull added a commit that referenced this issue Aug 11, 2023
@driskull
fix(scrim): render text content inside scrim (#7509)
643ce5d 
**Related Issue:** #7032

## Summary

- Use new dom utility to test for content
- Add stories
- Update tests
@driskull driskull added the 3 - installed Issues that have been merged to master branch and are ready for final confirmation. label Aug 11, 2023
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 0 - new New issues that need assignment. 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Aug 13, 2023
@geospatialem
Copy link
Member

Verified in 1.6.0-next.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Field Apps Issues logged by ArcGIS Field Apps team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 3 A day or two of work, likely requires updates to tests. has workaround Issues have a workaround available in the meantime. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - medium Issue is non core or affecting less that 60% of people using the library
Projects
None yet
Milestone
1.6.0 - Low risk issues only
Development

No branches or pull requests
4 participants
@jcfranco @nwhittaker @driskull @geospatialem