Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

fix: Fixes Modal using animation frame #230

Merged
merged 1 commit into from
Mar 7, 2019
Merged

Conversation

ChristianKienle
Copy link
Contributor

@ChristianKienle ChristianKienle commented Mar 6, 2019

So this is how I would fix the modal.

It is ugly. :D By using an animation frame / setTimeout, we simply delay the registration of event handlers.

There are other ways to do it. One would be to use the same approach using for FdPopover which we already discussed within the context of the FdPopper-component. But this is a minimally invasive fix. I also think that the use case is different:

I think a trigger component and a simple popover usually share the same context. There is usually not a lot of reuse. However I might have a single "Save Modal" which I want to trigger from everywhere. At least this was the reason for doing it differently. But we will see. If you think this change is acceptable simply merge it into your branch.

If not: Let's talk. :D

@ChristianKienle ChristianKienle marked this pull request as ready for review March 6, 2019 13:38
Copy link
Contributor

@suwarnoong suwarnoong left a comment

Choose a reason for hiding this comment

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

Looks good. I just learned how the prop.sync works. Awesome

@suwarnoong suwarnoong merged commit e840554 into test/modal Mar 7, 2019
@suwarnoong suwarnoong deleted the fix/modal-using-raf branch March 7, 2019 03:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants