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

Viewport events #4395

Merged
merged 12 commits into from Oct 26, 2020
Merged

Viewport events #4395

merged 12 commits into from Oct 26, 2020

Conversation

moniika
Copy link
Contributor

@moniika moniika commented Oct 22, 2020

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Part of #4203
Part of #1052

Proposed Changes

Creates dedicated class (Blockly.Events.ViewportChange) for viewport change events.

Reason for Changes

See issue, but generally motivated by:

  • better typing
  • clearer events

Test Coverage

Tested by logging events on playground and observing that expected events were logged and running mocha tests.

Tested on:

  • Desktop Chrome

Copy link
Contributor

@alschmiedt alschmiedt left a comment

Choose a reason for hiding this comment

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

this lgtm! I just have a few things I am curious about:

  1. Is there a use case in mind for this event?
  2. Depending on the use case, would it be helpful to have the width and size of the view port for this event?
  3. Do we nee the blockly_compressed file? I know we need blockly_uncompressed since you added a file, but not sure about blockly_compressed.

@moniika
Copy link
Contributor Author

moniika commented Oct 26, 2020

this lgtm! I just have a few things I am curious about:

  1. Is there a use case in mind for this event?
  2. Depending on the use case, would it be helpful to have the width and size of the view port for this event?
  3. Do we nee the blockly_compressed file? I know we need blockly_uncompressed since you added a file, but not sure about blockly_compressed.
  1. I can't remember the specific use case, but I've added the related bug
  2. I was also thinking width/height (size) might be useful, Maybe to follow up I can add.
  3. I had just made the assumption that it was also needed, but looks like it isn't necessary.

I'm going to leave for now, but may add viewport size information in a follow up.

@moniika moniika merged commit ea48707 into google:ui-events Oct 26, 2020
@moniika moniika deleted the viewport-events branch October 26, 2020 21:48
moniika pushed a commit that referenced this pull request Nov 4, 2020
* Ui events base (#4370)

* Add constants for new ui event types

* Add property to indicate an event as UI event

* Click events (#4372)

* Creating new ui base class.

* Refactor theme event (#4391)

* Add themeName property to theme event

* Refactor marker move events. (#4389)

* Refactor trashcan open event (#4392)

* Refactor selected event (#4387)

* Refactor toolbox item change event (#4394)

* Refactor bubble open events (#4390)

* Refactor block drag event (#4388)

* Viewport events (#4395)

* Fix event filtering for ui events (#4401)

* Move events to new directory and rename Ui events base (#4400)

* Move events to new directory and rename Ui events base

* Add missing fromJson implementation for click event (#4410)

* Adding serialization tests for events

* Zoom controls event (#4407)

* Refactor zoom event

* Rename IS_UI_EVENT to isUiEvent
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.

None yet

2 participants