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

[gui] refactors modality handling for dialogs #7428

Merged
merged 4 commits into from
Jul 13, 2015

Conversation

xhaggi
Copy link
Member

@xhaggi xhaggi commented Jul 6, 2015

This PR refactors the modality handling of dialogs out of the methods CGUIDialog::DoModal and CGUIDialog::Show. Now we specify the modality on a per dialog basis and the modality is not changed depending on the method call. Therefore I've dropped the methods DoModal and Show and introduced a new method CGUIDialog::Open as a replacement.

@Montellese @mkortstiege @Paxxi mind taking a look.

@xhaggi xhaggi added the Type: Improvement non-breaking change which improves existing functionality label Jul 6, 2015
@xhaggi xhaggi force-pushed the refactor-dialog-modality branch 2 times, most recently from cd7963b to d5c9376 Compare July 8, 2015 09:00
@mkortstiege
Copy link
Member

Look good and clean to me. I wonder if there's a scenario where a dialog has to be called with a specific modality though.

@xhaggi
Copy link
Member Author

xhaggi commented Jul 9, 2015

any other objections? @Paxxi or @Montellese

@xhaggi
Copy link
Member Author

xhaggi commented Jul 9, 2015

jenkins build this please

@xhaggi xhaggi added this to the Isengard 16.0-alpha1 milestone Jul 9, 2015
@Paxxi
Copy link
Member

Paxxi commented Jul 10, 2015

@xhaggi I haven't gone through it in detail but I like the changes and cleanup and I think it's going in the right direction, 👍

@xhaggi
Copy link
Member Author

xhaggi commented Jul 10, 2015

ok i'm waiting for some feedback from @Montellese and if he's fine we should shove it in asap.

//send message and wait for user input
ThreadMessage tMsg = {TMSG_DIALOG_DOMODAL, WINDOW_DIALOG_YES_NO, g_windowManager.GetActiveWindow()};
CApplicationMessenger::Get().SendMessage(tMsg, true);
pDialog->Open();

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@Montellese
Copy link
Member

As mentioned in the commit comment I'm not 100% sure about all the changes from (blocking) threaded messages to directly opening the dialog since it changes the thread in which the dialog is opened. But I also don't know the guilib code that well.

@xhaggi
Copy link
Member Author

xhaggi commented Jul 12, 2015

apart from your comments any other objections?

@Montellese
Copy link
Member

I don't think so.

@xhaggi xhaggi force-pushed the refactor-dialog-modality branch 2 times, most recently from 520bbc8 to 01e2dce Compare July 13, 2015 07:28
@xhaggi
Copy link
Member Author

xhaggi commented Jul 13, 2015

jenkins build this please

xhaggi added a commit that referenced this pull request Jul 13, 2015
[gui] refactors modality handling for dialogs
@xhaggi xhaggi merged commit 76f2a71 into xbmc:master Jul 13, 2015
@xhaggi xhaggi deleted the refactor-dialog-modality branch July 18, 2015 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants