Alert placed in the position of first opened window #119

Closed
ProvGANG opened this Issue Apr 19, 2015 · 15 comments

Comments

Projects
None yet
3 participants
@ProvGANG

When I open a new window or load it from my app, alert widget is shown in the middle of parent window. This changes when the parent window is closed but there are situations when I want both of them running. Here's the code sample

Shoes.app do
    button "new" do
        window do
            button "alert!" do
                alert("OK!")
            end
        end 
    end
end

alert

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Apr 19, 2015

Contributor

If i understand you correctly, you want the alert dialog to always pop up over the window who launched it, while this would be easy to implement it means breaking backward compatibility and apparently a design feature (all dialogs are acting this way), not a big threat, compatibility wise, though, i think.
I always found this a bit annoying, when working on second window, the popup makes you switch automatically to the main window, you'll have to go back .... etc

opinions ?
if changing behavior, do we want this to happen only for ask, alert, confirm dialogs or also to open, save, color dialogs ?

Contributor

passenger94 commented Apr 19, 2015

If i understand you correctly, you want the alert dialog to always pop up over the window who launched it, while this would be easy to implement it means breaking backward compatibility and apparently a design feature (all dialogs are acting this way), not a big threat, compatibility wise, though, i think.
I always found this a bit annoying, when working on second window, the popup makes you switch automatically to the main window, you'll have to go back .... etc

opinions ?
if changing behavior, do we want this to happen only for ask, alert, confirm dialogs or also to open, save, color dialogs ?

@ProvGANG

This comment has been minimized.

Show comment
Hide comment
@ProvGANG

ProvGANG Apr 19, 2015

As you said, it's quite annoying and I see no advantage of the way it is working now.

As you said, it's quite annoying and I see no advantage of the way it is working now.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 20, 2015

Contributor

There is also the situation of two identically sized windows - one hides the other and as noted by @BackOrder , there is no placement options for the first window either - perhaps you don't want it in the center of the screen.

When you open up the APP/Windows structure for work or redesign for this bug there's some other requests on the issues list that need consideration to make sure those requests are not closed off by a hasty bug fix.

There is a case for the current situation - alerts/dialogs belong to the running Shoes C level application and not to the current script or a block in a script. It's one of those design decisions made long ago to keep Shoes simple and cross platform. That's were you start running into backwards compatibility issues. Remember, Shoes 4 is supposed to be compatible with Shoes 3.

Two perspectives from one person.

Contributor

ccoupe commented Apr 20, 2015

There is also the situation of two identically sized windows - one hides the other and as noted by @BackOrder , there is no placement options for the first window either - perhaps you don't want it in the center of the screen.

When you open up the APP/Windows structure for work or redesign for this bug there's some other requests on the issues list that need consideration to make sure those requests are not closed off by a hasty bug fix.

There is a case for the current situation - alerts/dialogs belong to the running Shoes C level application and not to the current script or a block in a script. It's one of those design decisions made long ago to keep Shoes simple and cross platform. That's were you start running into backwards compatibility issues. Remember, Shoes 4 is supposed to be compatible with Shoes 3.

Two perspectives from one person.

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Apr 20, 2015

Contributor

yes not so simple situation,
just for the sake of knowing what we are talking about,
the fix i tried is dead simple : a new macro in world.h ACTUAL_APP which, unlike GLOBAL_APP, gets the window who launched the dialog (via self, which is available in all dialogs methods) in place of the first opened (main).

#define ACTUAL_APP(appvar) \
  shoes_app *appvar = NULL; \
  if (RARRAY_LEN(shoes_world->apps) > 0) \
      Data_Get_Struct(self, shoes_app, appvar)

Then we can replace GLOBAL_APP by ACTUAL_APP on a method by method basis basis in gtk.c or cocoa.m (needs testing on osx, but i don't see why it wouldn't work)

looks it won't be troublesome for other issues on windows/app, and again dead simple to go back if needed

now for compatibility, the change is very light from a code perspective : it won't change anything in the deeps of Shoes, from a user point of view it's just a popup appearing differently, and it might do so in a less annoying way :-))
Like suggested we could do this only for alert/ask/confirm ...

a not so objective prospective point of view from my last 2 cents

Contributor

passenger94 commented Apr 20, 2015

yes not so simple situation,
just for the sake of knowing what we are talking about,
the fix i tried is dead simple : a new macro in world.h ACTUAL_APP which, unlike GLOBAL_APP, gets the window who launched the dialog (via self, which is available in all dialogs methods) in place of the first opened (main).

#define ACTUAL_APP(appvar) \
  shoes_app *appvar = NULL; \
  if (RARRAY_LEN(shoes_world->apps) > 0) \
      Data_Get_Struct(self, shoes_app, appvar)

Then we can replace GLOBAL_APP by ACTUAL_APP on a method by method basis basis in gtk.c or cocoa.m (needs testing on osx, but i don't see why it wouldn't work)

looks it won't be troublesome for other issues on windows/app, and again dead simple to go back if needed

now for compatibility, the change is very light from a code perspective : it won't change anything in the deeps of Shoes, from a user point of view it's just a popup appearing differently, and it might do so in a less annoying way :-))
Like suggested we could do this only for alert/ask/confirm ...

a not so objective prospective point of view from my last 2 cents

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 20, 2015

Contributor

Hard to argue against working code, @passenger94. Issue the pull request and I'll deal with the OSX side of things when I get a chance.

Contributor

ccoupe commented Apr 20, 2015

Hard to argue against working code, @passenger94. Issue the pull request and I'll deal with the OSX side of things when I get a chance.

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Apr 20, 2015

Contributor

test app

Shoes.app title: "Main" do
    button "new" do
        window title: "Deputy", width: 300, height: 200 do
            button("ask") {r = ask "do you ?"; alert "'#{r}'\nGlad to know !"}
            button("confirm") { r = confirm "sure ?"; alert "corroborated ?  #{r}" }
        end 
    end
end
Contributor

passenger94 commented Apr 20, 2015

test app

Shoes.app title: "Main" do
    button "new" do
        window title: "Deputy", width: 300, height: 200 do
            button("ask") {r = ask "do you ?"; alert "'#{r}'\nGlad to know !"}
            button("confirm") { r = confirm "sure ?"; alert "corroborated ?  #{r}" }
        end 
    end
end

@ccoupe ccoupe added the Normal label Apr 21, 2015

@ccoupe ccoupe added this to the 3.2.23 milestone Apr 21, 2015

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Apr 21, 2015

Contributor

OSX exhibits the old behavior, as expected. On Windows the new code behaves just as it does on Linux the alert/confirm pops up over the window with the button that launched it.

@ProvGANG - you can test it with the new beta http://walkabout.mvmanila.com/public/shoes/shoes-3.2.23-gtk2-32.exe if you like.

Contributor

ccoupe commented Apr 21, 2015

OSX exhibits the old behavior, as expected. On Windows the new code behaves just as it does on Linux the alert/confirm pops up over the window with the button that launched it.

@ProvGANG - you can test it with the new beta http://walkabout.mvmanila.com/public/shoes/shoes-3.2.23-gtk2-32.exe if you like.

@ProvGANG

This comment has been minimized.

Show comment
Hide comment
@ProvGANG

ProvGANG Apr 21, 2015

Thank you, that's great! 👍

Thank you, that's great! 👍

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Apr 21, 2015

Contributor

@ccoupe, ok i see now the cocoa side of the recipe ! not so sweet indeed !

Contributor

passenger94 commented Apr 21, 2015

@ccoupe, ok i see now the cocoa side of the recipe ! not so sweet indeed !

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Jul 16, 2015

Contributor

I've reverted the simple fix because it did cause backwards compatibility issues. #136. I think a better solution might be adding a new style "thing" that can the gtk or cocoa code can reference when creating windows. OSX has a NSWindow.setParentWindow method that might work (or not). Another option to consider is to implement the dialog class in Shoes - it's there but not really used and subclass ask/alert/confirm from shoes-dialog. That would mean duplicating some Ruby/C/Obj-C code to populate the subclass to do the right thing and of course that's not as simple as it sounds.

OSX dialog code is on my fix list (future deprecation issues) so I'll look into that side soon.

Contributor

ccoupe commented Jul 16, 2015

I've reverted the simple fix because it did cause backwards compatibility issues. #136. I think a better solution might be adding a new style "thing" that can the gtk or cocoa code can reference when creating windows. OSX has a NSWindow.setParentWindow method that might work (or not). Another option to consider is to implement the dialog class in Shoes - it's there but not really used and subclass ask/alert/confirm from shoes-dialog. That would mean duplicating some Ruby/C/Obj-C code to populate the subclass to do the right thing and of course that's not as simple as it sounds.

OSX dialog code is on my fix list (future deprecation issues) so I'll look into that side soon.

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Jul 16, 2015

Contributor

Bad news. As best as I can tell, on OSX a modal dialog (which is what alert.ask/confirm/ is) will be placed in the center of the Screen. Their rules. After thinking about it, Apple is correct - Dialogs go up front, in your face. Shoes.ask() should not be used for lazy edit lines if you care about layout and appearance. A patch to gtk.c (windows and linux) to put the dialogs in th center of the screen would be reasonable, IMO of course.

Contributor

ccoupe commented Jul 16, 2015

Bad news. As best as I can tell, on OSX a modal dialog (which is what alert.ask/confirm/ is) will be placed in the center of the Screen. Their rules. After thinking about it, Apple is correct - Dialogs go up front, in your face. Shoes.ask() should not be used for lazy edit lines if you care about layout and appearance. A patch to gtk.c (windows and linux) to put the dialogs in th center of the screen would be reasonable, IMO of course.

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Jul 17, 2015

Contributor

ok i think i have it

replace ACTUAL_APP macro by this one and it should work both ways (subclass or regular)

#define ACTUAL_APP(appvar) \
  shoes_app *appvar = NULL; \
  VALUE actual_app = rb_funcall2(self, rb_intern("app"), 0, NULL); \
  Data_Get_Struct(actual_app, shoes_app, appvar);

self refers indeed to the subclass in the url/visit scheme, so Data_Get_Struct(self, shoes_app, appvar); wasn't correct !!

tested with bug119.rb and #136 example
meanwhile we can explore the style option ...

side note @ccoupe : in gtk, alert and co are always placed at the center of the "calling" window

Contributor

passenger94 commented Jul 17, 2015

ok i think i have it

replace ACTUAL_APP macro by this one and it should work both ways (subclass or regular)

#define ACTUAL_APP(appvar) \
  shoes_app *appvar = NULL; \
  VALUE actual_app = rb_funcall2(self, rb_intern("app"), 0, NULL); \
  Data_Get_Struct(actual_app, shoes_app, appvar);

self refers indeed to the subclass in the url/visit scheme, so Data_Get_Struct(self, shoes_app, appvar); wasn't correct !!

tested with bug119.rb and #136 example
meanwhile we can explore the style option ...

side note @ccoupe : in gtk, alert and co are always placed at the center of the "calling" window

passenger94 added a commit to passenger94/shoes3 that referenced this issue Jul 18, 2015

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Jul 18, 2015

Contributor

Yes! You did fix it. Thank you! I changed the macro name (for now) but its working on my Linux and Windows, both #136 and #119. Neither of us is going to change gtk or cocoa dialog behavior. We just do what we can. Not sure what to do about gtk.c:973 (but it works so maybe nothing needs to be done).

Contributor

ccoupe commented Jul 18, 2015

Yes! You did fix it. Thank you! I changed the macro name (for now) but its working on my Linux and Windows, both #136 and #119. Neither of us is going to change gtk or cocoa dialog behavior. We just do what we can. Not sure what to do about gtk.c:973 (but it works so maybe nothing needs to be done).

@passenger94

This comment has been minimized.

Show comment
Hide comment
@passenger94

passenger94 Jul 18, 2015

Contributor

hi :-)
looks like gtk.c:973 is OK to me, i would say since it's the start of the main loop, only global_app is concerned ...

Contributor

passenger94 commented Jul 18, 2015

hi :-)
looks like gtk.c:973 is OK to me, i would say since it's the start of the main loop, only global_app is concerned ...

@ccoupe

This comment has been minimized.

Show comment
Hide comment
@ccoupe

ccoupe Aug 12, 2015

Contributor

Fixed in 3.2.24

Contributor

ccoupe commented Aug 12, 2015

Fixed in 3.2.24

@ccoupe ccoupe closed this Aug 12, 2015

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