Skip to content

fix(modal): correct undetached positioning when large modal is used #1096

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

Merged

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Oct 15, 2019

Description

This PR enhances the fix by @prudho and corrects positioning calculation when detachable: false was used together with large modals.
This now works in any possible combination: detachable true/false, useFlex: true/false, top aligned, bottom aligned

Testcase

http://jsfiddle.net/5er1f3va/2/

Closes

#1079
#1138
Semantic-Org/Semantic-UI#4676

@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug lang/css Anything involving CSS lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews labels Oct 15, 2019
@lubber-de lubber-de added this to the 2.8.0 milestone Oct 15, 2019
Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

LGTM

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

Copy link
Member

@ColinFrick ColinFrick left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

LGTM, this solution seems more robust.

@y0hami y0hami removed the state/awaiting-reviews Pull requests which are waiting for reviews label Oct 18, 2019
@y0hami y0hami changed the title [Modal] Corrected undetached positioning when large modals are used fix(modal): correct undetached positioning when large modal is used Oct 18, 2019
@y0hami y0hami merged commit 357da97 into fomantic:develop Oct 18, 2019
@lubber-de lubber-de deleted the fix/1079/detached_modal_positioning branch October 18, 2019 07:48
lubber-de added a commit that referenced this pull request Feb 16, 2020
…with detachable false

This PR gives full control over the useFlex setting again.
Some combinations are not working for correct positioning, which was basically fixed by #1079 and #1096
However, the fixes ignored a possible explicit given useFlex:true and always assumes auto instead (which makes sure the positioning will work all the time)
As it turned out here there are some usecases, where people need the explicit setting of useFlex:true, i reverted the logic for the can.useFlex method but added some debug warnings instead when unsupported combinations occur, because the positioning for useFlex:true will not work when:

- IE11 is used
- detachable:false is used
As the default of useFlex is auto, existing code should basically work as before without any visual issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/css Anything involving CSS lang/javascript Anything involving JavaScript tag/sui-issue Taken from an existing Issue/PR of SUI 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.

5 participants