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

UI Event Handling API #2253

Closed
BeksOmega opened this issue Feb 3, 2019 · 3 comments
Closed

UI Event Handling API #2253

BeksOmega opened this issue Feb 3, 2019 · 3 comments
Labels
component: events issue: feature request Describes a new feature and why it should be added

Comments

@BeksOmega
Copy link
Collaborator

BeksOmega commented Feb 3, 2019

Problem statement

It's hard to add UI objects ontop of the workspace. It's also hard to listen to workspace move/resize events. It's especially hard if you don't want to mess with the core.

Expected Behavior

There should be a way to add extentions to the workspace in a way that doesn't affect the core.

Proposal

-=-=-=-=-=-=-=-=-=-=-=-=-=-
See this for updated proposal.
-=-=-=-=-=-=-=-=-=-=-=-=-=-

If this API were to be created I think it should ideally support at least two things:

  1. Adding UI Elements ontop of the workspace.
  2. Adding alternative navigation models (e.g. a minimap)

This proposal is very WIP, and you guys have probably already thought about it, but I wanted to post anyway. Here's the bad version:

Add some new workspace functions. (basically just adding in two subscriber-provider patterns)

  • addLayoutSubscriber(listener)
  • removeLayoutSubscriber(listener)
  • addContentChangeSubscriber(listener)
  • removeContentChangeSubscriber(listener)

The layout subscribers would have a layout(metrics) function called on them whenever they need to be re-layed-out, i.e. when the window is resized, just like how the resize() function works now. This supports goal 1.

Edit2: The layout subscribers would also need a setVisible(visibility) function, so that they could hide along with the workspace.

The content change subscribers would have a contentChanged(metrics) function called on them whenever the 'content' of the workspace changes, e.g. zooming, scrolling, blocks added (although this calls scroll so still mostly zooming and scrolling). This supports goal 2.

The advantages of both of these are A) it makes blockly more easily extendable and B) we can cache the getMetrics() values before we talk to the change subscribers C) things like the toolbox, scrollbars, trashcan, etc could be separated from the workspace, making the system more flexible & readable.

Edit: For goal 2 there should also probably be a "standard" way for navigation elements to talk to the workspace. I suggest either the public scroll/pan/zoom APIs purposed by @AnmAtAnm or a system of using percentage values (e.g. scrollTo(0.5, 0.5)).

Edit3: When I originally planned this I didn't mention using the events system because I though the 10ms delay would cause visible lag, but then I realized that I don't know how browsers render frames, so maybe the event system could work for this.

I am willing to impliment this if you guys think it is a good idea, if not that's cool too. I had fun thinking about it and writing it up =)

@AnmAtAnm
Copy link
Contributor

AnmAtAnm commented Feb 5, 2019

Can you reframe this in terms of what is missing from the Blockly Events API and the UI events. I think we're open to expanding the supported events, but we want to continue to use the existing model and infrastructure.

One area where we definitely need improvement is what fields an event object should include for each element subtype.

@AnmAtAnm AnmAtAnm added issue: feature request Describes a new feature and why it should be added component: events labels Feb 5, 2019
@BeksOmega
Copy link
Collaborator Author

we want to continue to use the existing model and infrastructure.

Makes sense!

Can you reframe this in terms of what is missing from the Blockly Events API and the UI events.

Ok I gave it a shot!

Problem Statement 1

At the moment there are no events fired when the workspace is resized, or when the workspace's visibility changes.

This makes it hard to create UI elements that exist ontop of the workspace. All of the objects that do exist on top of the workspace are referenced by the workspace individually, and have their position, and setVisible functions called "manually".

Problem Statement 2

At the moment there are no events fired when the workspace is A) scrolled B) zoomed or C) when it's content is changed (e.g. block movement).

This makes it hard to create custom navigation components, as you can see when you look at the minimap demo.

Proposal V2 (here's the second bad version)

I believe a WORKSPACE_LAYOUT event should be fired in place of, or in addition to the workspace.resize() function. This would include a "workspaceMetrics" property in addition to the default properties. This would help fix Problem 1, as well as allow the metrics to be cached, improving performance.

I believe a WORKSPACE_VIBILITY event should be fired in place of, or in addition to the workspace.setVisible() function. This would include a "visible" property indicating if the workspace is now visible or not in addition to the default properties. This would help fix Problem 1.

I believe a WORKSPACE_CONTENT_CHANGED event should be fired whenever the workspace is scrolled (which would by extension fire it whenever the workspace is zoomed) as well as whenever the 'content' of the workspace is changed (content in this case basically means the size of the block bounding box). This would include a "workspaceMetrics" property in addition to the default properties. This would help fix Problem 2, as well as allow the metrics to be cached, improving performance (not that that's really an issue with only 1 navigation component :P ).

I hope that's a little bit more understandable than the first version (if not more succinct lol).

@BeksOmega
Copy link
Collaborator Author

Found another issue about this by rachel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: events issue: feature request Describes a new feature and why it should be added
Projects
None yet
Development

No branches or pull requests

3 participants