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

Make palettes behave like sugar-toolkit. #129

Closed
wants to merge 1 commit into from

Conversation

icarito
Copy link

@icarito icarito commented Sep 22, 2017

They will close when clicking anywere else.

@llaske you may be interested in this one. Makes palettes less frustrating.

They will close when clicking anywere else.
@icarito
Copy link
Author

icarito commented Sep 22, 2017

fixes #126

Copy link

@AllenAby AllenAby left a comment

Choose a reason for hiding this comment

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

The function to close the palette seems to be correct, when the instance method is called on the palette object, it will become hidden upon clicking outside the palette

document.body.removeEventListener("click", closePalette);
window.removeEventListener("blur", closePalette);
}
setTimeout( function() {
Copy link
Member

Choose a reason for hiding this comment

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

Why is this done inside a timeout with value 0?

@sanatankc
Copy link

Is there something I can do. It was in GCI tasks.

@walterbender
Copy link
Member

It would be good to have a clearer statement of what this patch does. And an answer to my earlier question.

@quozl
Copy link
Contributor

quozl commented Jan 5, 2018

I've reviewed;

I've tested the palette or tooltip hide response of GTK+ 2, GTK+ 3, and JavaScript toolkits using activities Moon, Chat, and Gears, on the Sugar desktop. Tooltips correctly hide, but I was not able to find an example of palettes in a JavaScript activity.

I think icarito was onto something, and a fix would be welcome, but as he has withdrawn it will be up to our other developers to champion the change, and work with @walterbender to get it merged.

So, @sanatankc or @ccr4b, can you give us some help;

  • a test case of a JavaScript activity running on Sugar desktop that shows palettes not hiding when mouse leaves them,
  • if the proposed patch fixes the problem,
  • why the proposed patch implemented a fix using a timeout rather than immediate,
  • any alternate patch.

@cheongsiuhong
Copy link

@quozl regarding your comments - I've done some testing regarding this fix by doing the following:

  1. Setting up sugarizer's Moon.activity in sugar desktop.
  2. Replacing the sugar-web library with the one from this repository.

As stated in #126, the journal palette does not close when the mouse moves outside nor when the mouse clicks outside the palette.

The fix implemented in this PR works as stated, and the palette closes when mouse clicks occur anywhere outside the palette. The fix works by adding click event listener when the palette is popped up; a click event would then trigger the palette close function.

Without the timeout, the click event would trigger the palette close function on the same click that opens the palette, and as a result the palette cannot open at all. The fix is fairly hacky though it does do its job, it is not clear if it is really needed. I've opened a few native sugar and sugarizer activities and it appears that palettes have the same behavior as described in #126; they do not close when the mouse moves or clicks outside them. I am not too sure if ALL native sugar activity palettes behave like this though, please do correct me if I'm wrong.

Otherwise, I would suggest that this palette behavior be left alone and the issues and pull request to be closed.

@quozl
Copy link
Contributor

quozl commented Mar 29, 2020

Interesting, thanks.

I've just tested Gears on Ubuntu 20.04, and #126 continues as you say.

Looking at how Python GTK activities behave;

  • the activity icon when clicked resizes the canvas, shows a toolbar under the main toolbar, and the description button shows a palette. Both are not dismissed by moving the mouse or clicking outside the object.
  • the activity icon when approached using mouse pointer shows a palette toolbar, and the description button shows a palette. Both are dismissed by moving the mouse or clicking outside the object.

Gears and sugar-web I guess does not support moving the mouse pointer into the object to activate. Only clicks will work?

@icarito
Copy link
Author

icarito commented Mar 30, 2020

Hi,
I'd forgotten about this PR.
I don't remember the precise circumstances at the time, but I do remember that by the time Walter asked, I'd forgotten what the timeout was for. Looking at the code right now it is not completely apparent to me either but @cheongsiuhong analysis seems right.

@icarito
Copy link
Author

icarito commented Mar 30, 2020

Looking at how Python GTK activities behave;

* the activity icon when clicked resizes the canvas, shows a toolbar under the main toolbar, and the description button shows a palette.  Both are not dismissed by moving the mouse or clicking outside the object.
* the activity icon when approached using mouse pointer shows a palette toolbar, and the description button shows a palette.  Both are dismissed by moving the mouse or clicking outside the object.

Gears and sugar-web I guess does not support moving the mouse pointer into the object to activate. Only clicks will work?

I'm not sure I understand what you are asking. I just went to try.sugarizer.org and tried Gears. Mouse pointer behaviour is: style change "on hover" (dark button) and the hover tooltip title. I observe the same with other activities. "click" is a fairly broad event that triggers on any pointer, I believe.

The reason I implemented this was for one thing consistency but also I observed frustration from our users here at home that the menus seemed to "stay on". I should've mentioned all this in the PR / commit message.

Regards and thanks @cheongsiuhong !

@quozl
Copy link
Contributor

quozl commented Mar 30, 2020

I'm not sure I understand what you are asking.

On Ubuntu 20.04 with Sugar 0.117 and Gears; on hover is style change and tooltip, as you say. But same platform using a Python GTK activity responds by opening the palette. The difference in response is unexpected, and I don't know which response is the right one to plan for in future.

@icarito
Copy link
Author

icarito commented Mar 30, 2020 via email

@quozl
Copy link
Contributor

quozl commented Mar 30, 2020

I expect as it would be a user experience breaking-change for Sugarizer, that hover won't be added. @llaske, please confirm you have no plans for this? Only reason I can think of is user experience similarity with Sugar, and that's not a requirement I foresee.

@cheongsiuhong
Copy link

cheongsiuhong commented Mar 30, 2020

Hi all, I'm currently experimenting on it but pop up on hover can be implemented by adding an event listener for "mouseover". Shall I open a pull-request and work it out from there?

edit: Also, this current implementation is bugged! If you were to click on the palette itself (to type the activity name, or something), it would close the palette! Probably not the intended behavior.

@quozl
Copy link
Contributor

quozl commented Mar 30, 2020

I expect implementing would be easy. It's the design and user experience issue that we must resolve first. I'll wait to hear from @llaske. Only a small group is interacting here. We could also widen the discussion to the mailing list, but if @llaske isn't interested, I don't think it would help to do so.

@cheongsiuhong
Copy link

Given that sugar-web for sugarizer has diverged quite a fair bit and is no longer similar to the one in this repository, implementing changes in both versions of sugar-web might be an issue.

However, I have managed to implement hover with just changes in palette.js. A diff of palette.js from both native sugar-web and sugarizer sugar-web shows that they are both still identical and thus implementing the fix in both versions should be trivial.

If @llaske is interested in making the change, I would be more than happy to provide the code.

In my opinion, since both sugar and sugarizer are mostly synonymous in their purpose and design concept, they should both adhere to a similar design with regards to the user experience as much as possible. However, I have only recently started to look into this project, so take what I say with a grain of salt.

@llaske , your input would be useful on this matter, as it could affect the course of future works on the sugar-web library.

@llaske
Copy link
Collaborator

llaske commented Mar 30, 2020

Sugarizer targets not only computers, it targets also devices with touch screen.
So we can't rely on a "mouseover" event to change the state of a palette.So for being consistent between platforms supported by Sugarizer, I think it's better to wait for an explicit click/touch to open/close a palette.

@quozl
Copy link
Contributor

quozl commented Mar 30, 2020

Thanks. Same with Sugar; a palette opened with a click cannot be closed by mouseover, and vice versa.

@quozl quozl closed this Mar 30, 2020
@quozl quozl mentioned this pull request Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants