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

[Modal] Fix #6198 - added check whether focused element is in a viewport #6281

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

inmylo
Copy link

@inmylo inmylo commented Mar 19, 2018

I believe there is no need to restore focus to the invisible element when modal window is closed. Then there won't be cases like #6198

I believe there is no need to restore focus to the invisible element.
@y0hami
Copy link
Member

y0hami commented Mar 19, 2018

I think this should be optional since some people might want to go back to the focused element.

@inmylo
Copy link
Author

inmylo commented Mar 19, 2018

This PR doesn't disable going to the focused element if it is visible. But if you want add a general flag to return or not to return to the invisible element - still good for me.

@y0hami
Copy link
Member

y0hami commented Mar 19, 2018

Ye, I think it should be an option so it doesn't change any existing behaviour.

@inmylo
Copy link
Author

inmylo commented Mar 19, 2018

If I add new setting to the $.fn.modal.settings = { at the end of the file - will I be able to configure it both globally with:

$.fn.modal.settings.focusInvisible = false;

and for the specific modal window with:

$('.ui.modal').modal({
  focusInvisible: false,
});

?
I ask because I'm not sure whether you set/append such settings somewhere else in other files.

@y0hami
Copy link
Member

y0hami commented Mar 19, 2018

From my knowledge the settings which are passed when calling the module override the global settings which are defined at the end of the file so yes if you do

$.fn.modal.settings.focusInvisible = false;
$('.ui.modal').modal({
  focusInvisible: true
});

The modal with .ui.modal will be true but all others will be false.

@inmylo
Copy link
Author

inmylo commented Mar 19, 2018

Thanks for the idea, I've added needed changes. Now it is optional and enabled by default.

@inmylo
Copy link
Author

inmylo commented May 2, 2018

Hello!

Can somebody merge this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants