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

Closed floating elements should not cause scrollbars #10240

Closed
2 of 6 tasks
driskull opened this issue Sep 6, 2024 · 6 comments
Closed
2 of 6 tasks

Closed floating elements should not cause scrollbars #10240

driskull opened this issue Sep 6, 2024 · 6 comments
Assignees
@geospatialem @DitwanP
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. breaking change Issues and pull requests with code changes that are not backwards compatible. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - low Issue is non core or affecting less that 10% of people using the library
Milestone
2024-11-26 - 3.0....

Comments

@driskull
Copy link
Member

driskull commented Sep 6, 2024 •
edited
Loading

Check existing issues

Actual Behavior

Closed floating elements cause scrollbars

Expected Behavior

Closed floating elements should not cause scrollbars

Reproduction Sample

https://codepen.io/driskull/pen/RwzEMYv?editors=1100

Reproduction Steps

  1. open the codepen
  2. notice the scrollbars shown due to a closed popover

Reproduction Version

2.12.2

Relevant Info

https://devtopia.esri.com/WebGIS/arcgis-js-api/issues/63967

Regression?

No

Priority impact

impact - p3 - not time sensitive

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Maps SDK for JavaScript
@driskull driskull 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 Sep 6, 2024
@github-actions github-actions bot added ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. calcite-components Issues specific to the @esri/calcite-components package. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive labels Sep 6, 2024
@driskull driskull mentioned this issue Sep 6, 2024
Merged
@geospatialem geospatialem added p - low Issue is non core or affecting less that 10% of people using the library 2 - in development Issues that are actively being worked on. estimate - 3 A day or two of work, likely requires updates to tests. and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 6, 2024
@driskull
Copy link
Member Author

@geospatialem @DitwanP moved this one to October.

@zaramatheson
Copy link

@driskull @geospatialem Any chance this one could get priortized for a 2.13 patch? It affects a few spots in MV. We are investigating the workaround desccribed in the JSAPI issue, but it would have to be implemented in quite a fewplaces.

@driskull
Copy link
Member Author

driskull commented Oct 3, 2024

@zaramatheson let me know if the workaround is working for you. I'll defer to @jcfranco and @geospatialem for when this can be installed.

@driskull
Copy link
Member Author

driskull commented Oct 3, 2024

It could be a separate issue than this one.

From the screenshot gifs, it seems like its more likely a flex/display issue. Something isn't taking up its full height which is causing the multiple scroll bars.

Its hard to be sure without a good reproduction case we can use.

If it is the position: relative on the flow-item or panel, you can easily override it by setting a style on the element. That style is applied to the host of the component so it can be overriden.

Can you try setting position:static or position:inherit to see if it fixes your use case?

@geospatialem geospatialem added ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. and removed ArcGIS Map Viewer Issues logged by ArcGIS Map Viewer team members. labels Oct 7, 2024
@iyouth23 iyouth23 mentioned this issue Oct 9, 2024
Closed
6 tasks
driskull added a commit that referenced this issue Oct 31, 2024
@driskull
fix!: floating-ui elements no longer take up space when closed (#10241)
91d9b2e 
**Related Issue:** #10240

## Summary

- Sets floating element dimensions to zero when not open
- This makes sure that a closed floating-ui element doesn't produce
scrollbars within a parent element.
- This is handled by setting the style on the floating element container
to `display:none` when not open.
- [Popover, Tooltip]: Moves position styles from the host to an internal
element to avoid style clashes.
- This is better because the positioning styles won't interfere with any
light DOM styling
  - This added benefit is less CSS in the floating-ui CSS file. 
- `display:none` is removed in the `onClose` of the component so that
the animation can finish before its set to be hidden.
  - This restores closing animation in floating ui elements.
- Cleanup floating-ui methods 
- Add story
- Update tests

Avoids closed floating ui elements from affecting layout changes.
Example:

<img width="272" alt="image"
src="https://github.com/user-attachments/assets/c4366635-8106-4e7c-b4d5-9b5ca03ae1d7">
@driskull driskull added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Oct 31, 2024
@github-actions GitHub Actions
Copy link
Contributor

Installed and assigned for verification.

@jcfranco jcfranco added the breaking change Issues and pull requests with code changes that are not backwards compatible. label Nov 4, 2024
@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 Nov 6, 2024
@geospatialem
Copy link
Member

Verified in 3.0.0-next.7

@geospatialem geospatialem mentioned this issue Nov 12, 2024
Closed
6 tasks
@jcfranco jcfranco mentioned this issue Nov 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

@DitwanP DitwanP

Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Maps SDK for JavaScript Issues logged by ArcGIS SDK for JavaScript team members. breaking change Issues and pull requests with code changes that are not backwards compatible. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components Issues specific to the @esri/calcite-components package. estimate - 3 A day or two of work, likely requires updates to tests. impact - p3 - not time sensitive User set priority impact status of p3 - not time sensitive p - low Issue is non core or affecting less that 10% of people using the library
Projects
None yet
Milestone
2024-11-26 - 3.0.0 Nov Breaking Chang...
Development

No branches or pull requests
5 participants
@jcfranco @driskull @geospatialem @zaramatheson @DitwanP