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

Move events to new directory and rename Ui events base #4400

Merged
merged 6 commits into from Oct 26, 2020

Conversation

moniika
Copy link
Contributor

@moniika moniika commented Oct 24, 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

Proposed Changes

  • Move events into events directory
  • Rename base class for UI to UiBase
  • Rename old ui events class back to Ui
  • Moves Ui class into same file as UiBase
    Creates dedicated class (Blockly.Events.ViewportChange) for viewport change events.

Reason for Changes

  • Organization
  • Backwards compatibility

Test Coverage

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

Tested on:

  • Desktop Chrome

goog.require('Blockly.registry');
goog.require('Blockly.utils.object');

/**
* Class for a UI event.
Copy link
Contributor

Choose a reason for hiding this comment

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

assuming i understand these changes correctly, i might add 'base class for a ui event, use the more specific event type in core/events if possible' or some other text explaining. because if i understand correctly this isn't meant to be used directly (?)

and same below for the plain Ui event, maybe add that it's there for compatibility but you should use the new event classes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've update Jsdoc for both and added @deprecated annotation to Events.Ui

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding @deprecated is an issue for travis until I also fix zoom event that is still using old class. Since I want to handle that in a seperate PR, I'm just going to update the jsdoc here.

@moniika moniika merged commit 3238600 into google:ui-events Oct 26, 2020
@moniika moniika deleted the cleanup branch October 26, 2020 22:17
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