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.

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

fix(modal,flyout): dynamic elements did not reference #2477

Merged
merged 7 commits into from
Oct 5, 2022

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Oct 3, 2022

Description

Whenever a flyout or modal were dynamically created, the internal element has to be reassigned, otherwise it would always be body. This is especially important when mutationobservers or this-references in callbacks are used

@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews labels Oct 3, 2022
@lubber-de lubber-de added this to the 2.9.0 milestone Oct 3, 2022
exoego
exoego previously approved these changes Oct 3, 2022
Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

LGTM

@mvorisek
Copy link
Contributor

mvorisek commented Oct 3, 2022

in Firefox this is causing a drag issue - when dragged a little bit, change is observed and the related refresh is causing the drag to stop, literally making any longer drag impossible

the problem is much more evident on 2nd or more drag, then the drag is stopped almost immediately

the fixed fiddle https://jsfiddle.net/lubber/jk31bLhg/ is also breaking modal in Microsoft Edge completely

@lubber-de
Copy link
Member Author

lubber-de commented Oct 3, 2022

@mvorisek
I am debugging this right now.

in Firefox this is causing a drag issue - when dragged a little bit, change is observed and the related refresh is causing the drag to stop, literally making any longer drag impossible

What kind of "drag" is affected? Could you provide an example? I already figured out that ff indeed breaks when the textarea of the given example jsfiddle reaches the viewport edge and gets stuck. This is caused by the scrolling class being set to modal and dimmer (still debugging how to fix it)
But from what you are mentioning, it seems to be something different you are "dragging"

the problem is much more evident on 2nd or more drag, then the drag is stopped almost immediately

Please provide a jsfiddle what/how you a dragging for reproducing. thx

the fixed fiddle jsfiddle.net/lubber/jk31bLhg is also breaking modal in Microsoft Edge completely

"completely" -> Please explain . I already figured out and fixed the input refresh by 702ec8b but beside that it works in Edge . So what is breaking "completely" for you? Could you provide a jsfiddle?

@mvorisek
Copy link
Contributor

mvorisek commented Oct 3, 2022

What kind of "drag" is affected?

by "drag" I mean dragging the bottom-right textarea corner (drag it vertically to stretch the textarea enough to enforce the Modal to be heigher than the browser viewport, then scrollbar on the right side of the browser is expected to be shown)

in Firefox, when I drag the textarea to stretch, the change seems to be observed almost immediately which causes the modal to be refresh and the drag is stopped - user cannot simply drag/stretch it in consequence

"completely" -> Please explain . I already figured out and fixed the input refresh by 702ec8b but beside that it works in Edge . So what is breaking "completely" for you? Could you provide a jsfiddle?

image

image

by "completely" I mean no modal is displayed at all, it seems attributeFilter: [ "style" ], is not supported by Edge

@lubber-de
Copy link
Member Author

lubber-de commented Oct 3, 2022

by "drag" I mean dragging the bottom-right textarea corner (drag it vertically to stretch the textarea enough to enforce the Modal to be heigher than the browser viewport, then scrollbar on the right side of the browser is expected to be shown)

in Firefox, when I drag the textarea to stretch, the change seems to be observed almost immediately which causes the modal to be refresh and the drag is stopped - user cannot simply drag/stretch it in consequence

ok, this only happens to me when the textarea gets too high while dragging, so are also referring to the textarea. thanks for clarification. i'll investigate

by "completely" I mean no modal is displayed at all, it seems attributeFilter: [ "style" ], is not supported by Edge

Ah...you meant old edge... already forgot about that :D ok, will fetch it . Thanks

@lubber-de
Copy link
Member Author

lubber-de commented Oct 4, 2022

I reverted my changes regarding additional attributes observer for now. I was not able to get this particular use case fixed. Even chrome beta seems to get annoyed when dragging the textarea. So lets check this later again. The remaining codechanges of this PR are still valid though

@lubber-de lubber-de merged commit b4c60c1 into fomantic:develop Oct 5, 2022
@lubber-de lubber-de deleted the refreshElementOnShortNotation branch October 5, 2022 18:17
@lubber-de lubber-de added hacktoberfest-accepted and removed state/awaiting-reviews Pull requests which are waiting for reviews labels Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted lang/javascript Anything involving JavaScript type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants