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

Convert Accessibility to TypeScript #6379

Merged
merged 13 commits into from
Feb 4, 2020
Merged

Conversation

Zyie
Copy link
Member

@Zyie Zyie commented Feb 1, 2020

Description of change

Converts accessibility to TypeScript.

Fairly simple conversion, just adding another mixin for DisplayObject and a new interface for an accessible HTMLElement.

I have removed the for loop that set its children's div to null in destroy. I couldn't find any example of div being set so i believe this is not needed?

Pre-Merge Checklist
  • Documentation is changed or added
  • Lint process passed (npm run lint)
  • Tests passed (npm run test)

@codecov-io
Copy link

codecov-io commented Feb 1, 2020

Codecov Report

Merging #6379 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #6379   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files         103      103           
  Lines        5840     5840           
=======================================
  Hits         4448     4448           
  Misses       1392     1392

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1c9cdb...b3f5c88. Read the comment docs.

@Zyie Zyie marked this pull request as ready for review February 2, 2020 19:44
@codecov-io
Copy link

codecov-io commented Feb 2, 2020

Codecov Report

Merging #6379 into dev will increase coverage by 0.69%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev   #6379      +/-   ##
=========================================
+ Coverage   76.01%   76.7%   +0.69%     
=========================================
  Files          80      76       -4     
  Lines        4356    4142     -214     
=========================================
- Hits         3311    3177     -134     
+ Misses       1045     965      -80

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e548c16...1fcfd00. Read the comment docs.

@pixijs pixijs deleted a comment from adireddy Feb 2, 2020

interactionManager.dispatchEvent(e.target.displayObject, 'mouseout', interactionManager.eventData);
// TODO: Remove casting when CanvasRenderer is converted
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes.


export interface IDestroyOptions {
children?: boolean;
texture?: boolean;
baseTexture?: boolean;
}

export interface DisplayObject extends InteractiveTarget, EventEmitter {}
export interface DisplayObject extends InteractiveTarget, IAccessibleTarget, EventEmitter {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another cross-module reference to check when we convert all the modules.

packages/accessibility/src/AccessibilityManager.ts Outdated Show resolved Hide resolved
@bigtimebuddy bigtimebuddy added this to the v5.3.0 milestone Feb 3, 2020
@ivanpopelyshev
Copy link
Collaborator

ivanpopelyshev commented Feb 3, 2020

@Zyie private shouldn't be readonly. That's why we want private field - no need for any in constructor and other places.

@Zyie
Copy link
Member Author

Zyie commented Feb 3, 2020

@Zyie private shouldn't be readonly. That's why we want private field - no need for any in constructor and other places.

Yeah my bad I popped the wrong stash without realising

@ivanpopelyshev
Copy link
Collaborator

Now you can change(this as any).isActive = true to this._isActive = true

@Zyie
Copy link
Member Author

Zyie commented Feb 3, 2020

Now you can change(this as any).isActive = true to this._isActive = true

Done

@ivanpopelyshev
Copy link
Collaborator

"Amputate left hand.
I said left.
I said hand!"

@Zyie Zyie mentioned this pull request Feb 3, 2020
3 tasks
@bigtimebuddy bigtimebuddy merged commit 9d4f584 into dev Feb 4, 2020
@bigtimebuddy bigtimebuddy deleted the dev-accessibilty-typescript branch February 4, 2020 00:42
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

5 participants