fix!: floating-ui elements no longer take up space when closed #10241

driskull · 2024-09-06T19:45:03Z

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:

…revent scrollbars

packages/calcite-components/src/utils/floating-ui.ts

jcfranco · 2024-09-09T19:46:03Z

Before reviewing, could you confirm the following issues are still handled by these changes?

#1381
#5694

I recall we moved away from -999...9px values in the past.

driskull · 2024-09-09T20:10:39Z

#1381 should not be an issue since we are using inset-inline which will not cause scrollbars in RTL direction.

#5694 seems to be an issue with this PR so I guess that would be a showstopper.

The problem with not positioning off screen is that it will affect the size of the parent container.

Maybe once we implement the dialog element and popover attribute it will not be an issue any longer.

For now, I'll close this PR since it causes issues with drag and drop and elements positioned offscreen.

jcfranco · 2024-09-09T20:14:10Z

Thanks for testing. Bummer about #5694. 😞

driskull · 2024-09-09T20:17:15Z

If I set the width and height to 0 instead of positioning it offscreen that seems to do the trick. I'll do some more testing.

packages/calcite-components/src/utils/floating-ui.ts

jcfranco

I have a few follow-up comments, but this is great, @driskull! ✨💪✨

packages/calcite-components/src/components/dropdown/dropdown.e2e.ts

packages/calcite-components/src/utils/floating-ui.spec.ts

…m:Esri/calcite-design-system into dris0000/floating-ui-position-when-closed

# Conflicts: # packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx # packages/calcite-components/src/components/popover/popover.e2e.ts

# Conflicts: # packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts # packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx

jcfranco

LGTM!

Could you update the PR title and description to match our breaking change formatting? For the breaking change footer, feel free to use notes from our 3.x. breaking change doc.

This can be merged after the Oct milestone wraps up.

jcfranco · 2024-09-16T17:26:42Z

packages/calcite-components/src/components/popover/popover.e2e.ts

 import { CSS } from "./resources";

 describe("calcite-popover", () => {
  describe("renders", () => {
-    renders("calcite-popover", { visible: false, display: "block" });
+    renders("calcite-popover", { display: "block" });
    renders(`<calcite-popover label="test" open reference-element="ref"></calcite-popover><div id="ref">😄</div>`, {


This can be done in a follow-up, but can you wrap one of these renders in a describe block? Nested tests need a separate block since our test helpers use the same names for describe/it calls.

jcfranco · 2024-09-16T21:53:21Z

packages/calcite-components/src/components/shell/shell.stories.ts

@@ -1848,6 +1848,13 @@ export const panelWithPopoverZIndex = (): string =>
          </calcite-flow-item>
        </calcite-flow>
      </calcite-shell-panel> </calcite-shell
-    ><calcite-popover open reference-element="button" offset-distance="-50" offset-skidding="15" style="z-index: 100">
+    ><calcite-popover
+      overlay-positioning="fixed"


Do you know why this is needed now?

Its not needed, ill revert the change. The panel inside the popover just needs a width here. Ill update the test.

packages/calcite-components/src/components/tooltip/tooltip.tsx

# Conflicts: # packages/calcite-components/src/components/dropdown/dropdown.e2e.ts

) **Related Issue:** #7537 ## Summary - depends on #10241 - updates `action` to allow cursor to be overriden - lists should set the `label` property for the "Move to" sorting menu to have a name for the list. - adds `calcite-sort-handle` component to handle sorting and moving between lists for non mouse users - internal component - adds stories - adds tests - list-item - deprecates `dragSelected` property: no longer needed. - deprecates `calciteListItemDragHandleChange` event. no longer needed. - removed setting `aria-posinset` and `aria-setsize`. These are only needed when only part of the items are availalbe in the DOM. So we can safely remove these. - https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-setsize - https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-posinset - replaces `calcite-handle` with `calcite-sort-handle` in the `list` component. - updates tests - adds tests - adds demo pages BREAKING CHANGE: Modifies the component's keyboard sorting experience by using a dropdown menu to move list items. The ListItem `dragSelected` property and `calciteListItemDragHandleChange` event have been removed due to no longer being relevant.

driskull added 2 commits September 6, 2024 12:01

fix: position floating-ui elements offscreen by default in order to p…

356a0d5

…revent scrollbars

add story

bce13df

github-actions bot assigned driskull Sep 6, 2024

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Sep 6, 2024

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

c4fde6e

driskull marked this pull request as ready for review September 6, 2024 19:47

driskull requested a review from jcfranco September 6, 2024 19:47

driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Sep 6, 2024

handle positioning with the util

0d41bc2

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 Sep 6, 2024

driskull commented Sep 6, 2024

View reviewed changes

packages/calcite-components/src/utils/floating-ui.ts Outdated Show resolved Hide resolved

driskull commented Sep 6, 2024

View reviewed changes

packages/calcite-components/src/utils/floating-ui.ts Outdated Show resolved Hide resolved

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

49c0b1e

driskull removed the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Sep 9, 2024

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

10c8e1a

driskull added 3 commits September 9, 2024 13:26

set height and width to zero when not open.

02ffbd4

cleanup

08b30f3

cleanup

5451874

driskull commented Sep 9, 2024

View reviewed changes

packages/calcite-components/src/utils/floating-ui.ts Outdated Show resolved Hide resolved

driskull added 5 commits September 9, 2024 14:16

cleanup

03090d3

WIP

2f2ef34

test

90131c9

testing

1556b4a

cleanup

1afd7eb

jcfranco approved these changes Sep 11, 2024

View reviewed changes

packages/calcite-components/src/components/dropdown/dropdown.e2e.ts Outdated Show resolved Hide resolved

packages/calcite-components/src/utils/floating-ui.spec.ts Outdated Show resolved Hide resolved

driskull added 2 commits September 26, 2024 14:19

Merge branch 'dris0000/floating-ui-position-when-closed' of github.co…

1df132c

…m:Esri/calcite-design-system into dris0000/floating-ui-position-when-closed

fix e2e

5e327f2

driskull added the breaking change Issues and pull requests with code changes that are not backwards compatible. label Oct 1, 2024

driskull marked this pull request as draft October 1, 2024 21:45

driskull mentioned this pull request Oct 7, 2024

feat(list)!: Support moving between different lists via keyboard #10480

Merged

driskull added 2 commits October 8, 2024 10:29

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

e5d0977

# Conflicts: # packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx # packages/calcite-components/src/components/popover/popover.e2e.ts

fix build

734f77c

jcfranco mentioned this pull request Oct 10, 2024

[Dialog] Some slotted content appears below or overlaps the header/footer #10095

Open

6 tasks

driskull modified the milestones: 2024-10-29 - Oct Milestone, 2024-11-19 - 3.0.0 Nov Breaking Change Release Oct 17, 2024

driskull added 2 commits October 23, 2024 14:35

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

4b22a1a

# Conflicts: # packages/calcite-components/src/components/input-date-picker/input-date-picker.e2e.ts # packages/calcite-components/src/components/input-date-picker/input-date-picker.tsx

fix tests

551249e

driskull marked this pull request as ready for review October 28, 2024 20:29

jcfranco approved these changes Oct 29, 2024

View reviewed changes

driskull changed the title ~~fix: floating-ui elements no longer take up space when closed~~ fix!: floating-ui elements no longer take up space when closed Oct 29, 2024

driskull added 2 commits October 29, 2024 10:11

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

910dc08

review fixes

b7f921c

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 Oct 31, 2024

fix story

2839cc2

driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Oct 31, 2024

Merge branch 'dev' into dris0000/floating-ui-position-when-closed

681a46e

# Conflicts: # packages/calcite-components/src/components/dropdown/dropdown.e2e.ts

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 Oct 31, 2024

cleanup stories

f740c9c

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 Oct 31, 2024

driskull merged commit 91d9b2e into dev Oct 31, 2024
13 checks passed

driskull deleted the dris0000/floating-ui-position-when-closed branch October 31, 2024 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: floating-ui elements no longer take up space when closed #10241

fix!: floating-ui elements no longer take up space when closed #10241

driskull commented Sep 6, 2024 •

edited

Loading

jcfranco commented Sep 9, 2024

driskull commented Sep 9, 2024

jcfranco commented Sep 9, 2024

driskull commented Sep 9, 2024

jcfranco left a comment •

edited

Loading

jcfranco left a comment •

edited

Loading

jcfranco Sep 16, 2024

driskull Oct 29, 2024

jcfranco Sep 16, 2024

driskull Oct 29, 2024

fix!: floating-ui elements no longer take up space when closed #10241

fix!: floating-ui elements no longer take up space when closed #10241

Conversation

driskull commented Sep 6, 2024 • edited Loading

Summary

jcfranco commented Sep 9, 2024

driskull commented Sep 9, 2024

jcfranco commented Sep 9, 2024

driskull commented Sep 9, 2024

jcfranco left a comment • edited Loading

Choose a reason for hiding this comment

jcfranco left a comment • edited Loading

Choose a reason for hiding this comment

jcfranco Sep 16, 2024

Choose a reason for hiding this comment

driskull Oct 29, 2024

Choose a reason for hiding this comment

jcfranco Sep 16, 2024

Choose a reason for hiding this comment

driskull Oct 29, 2024

Choose a reason for hiding this comment

driskull commented Sep 6, 2024 •

edited

Loading

jcfranco left a comment •

edited

Loading

jcfranco left a comment •

edited

Loading