Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

Select: need a way to disable the hide modifier from popper #783

Closed
mayank99 opened this issue Aug 18, 2022 · 7 comments
Closed

Select: need a way to disable the hide modifier from popper #783

mayank99 opened this issue Aug 18, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@mayank99
Copy link
Contributor

mayank99 commented Aug 18, 2022

We rely on popper's hide modifier to hide the popover when the reference element leaves the viewport.

There needs to be a way to disable this modifier either through popperOptions or through scoped styling.

Currently if a className is passed to Select, it gets applied on the select button. If popoverProps.className is passed, it gets applied on the menu. There is no way to target the tippy.

This was needed to work around a bug with App UI's modeless dialog. See sandboxes where this bug is found in the select inside the "modeless dialog":
https://www.itwinjs.org/sandboxes/MuthuKalamani/UIControlsInDialogBox
https://www.itwinjs.org/sandboxes/NancyMcCall/Override%20Dialog%20z-index

@mayank99 mayank99 added the enhancement New feature or request label Aug 18, 2022
@mayank99 mayank99 changed the title Select: no way to add a className to iui-popover Select: a way to add a className to iui-popover Aug 18, 2022
@mayank99 mayank99 changed the title Select: a way to add a className to iui-popover Select: need a way to disable the hide modifier from popper Aug 19, 2022
@mayank99
Copy link
Contributor Author

For more context: the data-reference-hidden attribute is being applied incorrectly because the modeless dialog sets width: 0; height: 0 to hide the backdrop.

https://github.com/iTwin/itwinjs-core/blob/b55e4ecff9423b54cfdfe65da159747438cd0195/ui/core-react/src/core-react/dialog/Dialog.scss#L36-L39

Ideal solution would be to improve the backdrop logic/styles in App UI, but I think it also makes sense for iTwinUI to offer a way to disable this hide plugin.

@r100-stack r100-stack self-assigned this Aug 24, 2022
@r100-stack
Copy link
Member

  • I might have understood issue incorrectly, but if this issue is looking for a way to hide a tippy, I think we can call instance.hide() (docs). Here instance is the variable storing the tippy instance.

image

  • We can also hide all the tippys on the page (docs)

Screenshot from 2022-08-24 08-33-05

  • We could also pass a prop to tippy to hideOnClick (docs)

image

  • Then there are also props for callbacks for onHidden ("once the tippy has been fully hidden and unmounted from the DOM") and onHide ("once the tippy begins to hide") (docs)

In the onHide prop,

You can optionally return false from this lifecycle to cancel a hide based on a condition.

The plugin's function fn returns an object of lifecycle hooks. Plugins are invoked per-instance and the plugin function definition takes the instance as an argument, so you can use private variables to create internal state in the plugin closure.

  • Using plugins, we can also hide on our own events, like hideOnEscape docs

@mayank99
Copy link
Contributor Author

mayank99 commented Aug 24, 2022

None of those are really what we need, except maybe the custom plugins.

What I'm talking about is popper's hide modifier (I have a link in the issue description) which basically does this: when the reference element (the select button) is scrolled out of the viewport, it hides the popover (the select menu).

And we need a way to disable this feature because it doesn't work in modeless dialog.

@mayank99
Copy link
Contributor Author

One option might be to use headless tippy and filter out the data-reference-hidden attribute based on a new prop. But I think this would require handling some of the other logic ourselves, so we would need to test it.

@r100-stack
Copy link
Member

From what I have searched, I don't think there is an easy way to disable popper's hide mechanism. Playing with the onHide() function callback to return false didn't work. As you suggested, one way is to handle the logic of when to add the data-reference-hidden attribute ourselves, but that involves work on our side. I will try one last time to find a way in the popper and tippy docs to disable this and update it here, but this is what I have tried so far.

@r100-stack r100-stack removed their assignment Aug 31, 2022
@mayank99
Copy link
Contributor Author

Closing this as we want to get rid of tippy.js completely.

@mayank99 mayank99 closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2022
@r100-stack r100-stack assigned r100-stack and unassigned r100-stack Nov 23, 2022
@r100-stack
Copy link
Member

Closing this as we want to get rid of tippy.js completely.

Until we do this, the itwinjs-core team have PR #4717 to change the way they hide the dialog backdrop. L37-39 fixes this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants