Skip to content

fix(shell): position side panels above the center content#9912

Merged
driskull merged 7 commits intodevfrom
dris0000/shell-panel-resize-z-index
Aug 1, 2024
Merged

fix(shell): position side panels above the center content#9912
driskull merged 7 commits intodevfrom
dris0000/shell-panel-resize-z-index

Conversation

@driskull
Copy link
Copy Markdown
Member

@driskull driskull commented Jul 30, 2024

Related Issue: #6658

Summary

  • position side panels above the center content
  • add screenshot test

@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jul 30, 2024
@github-actions github-actions Bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jul 30, 2024
@driskull driskull removed the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jul 31, 2024
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jul 31, 2024
@driskull driskull marked this pull request as ready for review July 31, 2024 22:47
jcfranco
jcfranco approved these changes Aug 1, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨🚀👨‍🚀🪐✨

Comment thread packages/calcite-components/src/components/shell/shell.stories.ts
@driskull driskull added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 1, 2024
@driskull driskull added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 1, 2024
@driskull
Copy link
Copy Markdown
Member Author

driskull commented Aug 1, 2024 •
edited
Loading

@jcfranco @macandcheese an issue I'm seeing is that if we make the panels a higher zindex than the center content, the existing tip manager would no longer be able to overflow and go outside of its container like it does in this screenshot

https://www.chromatic.com/test?appId=6266d45509d7eb004aa254fb&id=66ac13e797c819ac7db9e16d

What do you think?

@macandcheese
Copy link
Copy Markdown
Contributor

macandcheese commented Aug 1, 2024 •
edited
Loading

What do you think?

I think that's an expected result of the change - the Shell Panel shouldn't behave like that... I think a Dialog with placement="bottom" would replicate that use case if needed?

Edit - Tip Manager is deprecated anyway - maybe we can add an example that replicates the behavior with Dialog > Carousel - or potentially a note about setting z-index for those using the older components?

@driskull
Copy link
Copy Markdown
Member Author

driskull commented Aug 1, 2024

Ok, do you think this would be a breaking change though? Because currently we have styles specifically to tip-manager which places it absolutely above everything else. Should we maybe hold off on this one until sept main release?

@macandcheese
Copy link
Copy Markdown
Contributor

macandcheese commented Aug 1, 2024

I'm comfortable holding off for the breaking change release - we have Tip Manager slated for removal in v4 - but the breaking change notice should be sufficient?

@driskull driskull added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 1, 2024
@driskull
Copy link
Copy Markdown
Member Author

driskull commented Aug 1, 2024

NVM found a workaround. No breaking change 🎉

@driskull driskull merged commit 3c061b8 into dev Aug 1, 2024
@driskull driskull deleted the dris0000/shell-panel-resize-z-index branch August 1, 2024 23:43
@github-actions github-actions Bot added this to the 2.11.1 patch milestone Aug 1, 2024
@driskull driskull mentioned this pull request Aug 20, 2024
Closed
6 tasks
@benelan benelan mentioned this pull request Aug 28, 2024
Merged
@geospatialem geospatialem mentioned this pull request Sep 5, 2024
Closed
6 tasks
@driskull driskull mentioned this pull request Sep 16, 2024
Merged
driskull added a commit that referenced this pull request Sep 18, 2024
@driskull
fix(shell): fix z-index of slotted popover elements (#10325)
7fe1601 
**Related Issue:** #10214

## Summary

- fix css for z-index
  - should only set `z-index` on slotted `calcite-shell-panel`s
- add screenshot test
- regression from here:
#9912
@calcite-admin calcite-admin mentioned this pull request Sep 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcfranco jcfranco jcfranco approved these changes

Assignees

@driskull driskull

Labels

bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing.

Projects

None yet

Milestone

2024-08-27 - Aug Release

Development

Successfully merging this pull request may close these issues.

4 participants

@driskull @macandcheese @jcfranco @geospatialem