feat(ripple): convert to ts #4300

moog16 · 2019-01-25T22:48:33Z

related #4225

mdc-web-bot · 2019-01-25T23:04:41Z

All 758 screenshot tests passed for commit 75cb9ba vs. feat/typescript! 💯🎉

mdc-web-bot · 2019-01-25T23:49:39Z

All 758 screenshot tests passed for commit 97fed6f vs. feat/typescript! 💯🎉

mmalerba · 2019-01-25T23:53:40Z

packages/mdc-ripple/adapter.ts

 }

 export default MDCRippleAdapter;
+export {Point};


export {MDCRippleAdapter as default, MDCRippleAdapter, Point};

mmalerba · 2019-01-26T00:02:04Z

packages/mdc-ripple/adapter.ts

+
+  containsEventTarget(target: EventTarget | null): boolean;
+
+  registerInteractionHandler<K extends keyof GlobalEventHandlersEventMap>(


ooh fancy! I didn't know this was a thing :)

I'd suggest putting some aliases in a common place somewhere to make the types more readable:

type EventType = keyof GlobalEventHandlersEventMap; type SpecificEventListener<K extends EventType> = (evt: GlobalEventHandlersEventMap[K]) => void;

Then these would look like:

registerInteractionHandler<E extends EventType>(evtType: E, handler: SpecificEventListener<E>): void; registerResizeHandler(handler: SpecificEventListener<'resize'>): void;

yaa I like that 👍

mdc-web-bot · 2019-01-26T01:51:28Z

All 758 screenshot tests passed for commit 79542ed vs. feat/typescript! 💯🎉

mmalerba · 2019-01-28T19:24:38Z

packages/mdc-ripple/util.ts

+}
+
+interface CssWindow extends Window {
+  CSS?: CssObject;


TS has a type CSS which is equivalent to your CssObject, it's just not on the Window for some reason. You might be able to make a file of type fixes somewhere, like this:

declare interface Window { CSS: CSS; }

You could also add a fix for the msMatchesSelector thing, might be nicer than having to invent your own types and cast them everywhere

ya we need to invent a place for these global types. Maybe the correct place is in mdc-dom package. I'm going to add it to the index.ts for now. We can decide for a better place as this PR evolves.

Also I found that this works:

declare global { interface Window { CSS: CSS; } }

globals seems like a last ditch effort. Is that a valid pattern? Or is there a better way of accomplishing this?

mmalerba · 2019-01-29T00:26:07Z

typings/events.d.ts

+ * THE SOFTWARE.
+ */
+
+declare type EventType = keyof GlobalEventHandlersEventMap;


for these ones you might want to leave them in a .ts file and export them so users can use them too

codecov-io · 2019-01-29T00:30:13Z

Codecov Report

Merging #4300 into feat/typescript will decrease coverage by 0.11%.
The diff coverage is 93.46%.

@@                 Coverage Diff                 @@
##           feat/typescript    #4300      +/-   ##
===================================================
- Coverage            98.57%   98.46%   -0.12%     
===================================================
  Files                   92       92              
  Lines                 5674     5718      +44     
  Branches               760      768       +8     
===================================================
+ Hits                  5593     5630      +37     
- Misses                  81       88       +7

Impacted Files	Coverage Δ
packages/mdc-dom/ponyfill.ts	`100% <100%> (ø)`	⬆️
packages/mdc-ripple/foundation.ts	`100% <100%> (ø)`
packages/mdc-ripple/util.ts	`86.76% <78.04%> (ø)`
packages/mdc-ripple/index.ts	`98.21% <96.87%> (ø)`

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 c649de5...b675f5c. Read the comment docs.

moog16 · 2019-01-29T00:30:56Z

packages/mdc-menu-surface/adapter.ts

@@ -0,0 +1,131 @@
+/**


don't know how this ended up here...removing

mdc-web-bot · 2019-01-29T01:00:55Z

All 758 screenshot tests passed for commit 7abb9d2 vs. feat/typescript! 💯🎉

mmalerba · 2019-01-29T01:03:17Z

packages/mdc-ripple/adapter.ts

+
+  deregisterDocumentInteractionHandler<K extends EventType>(evtType: K, handler: SpecificEventListener<K>): void;
+
+  registerResizeHandler(handler: (evt: GlobalEventHandlersEventMap['resize']) => void): void;


SpecificEventListener<'resize'>

googlebot · 2019-01-29T06:15:07Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

moog16 · 2019-01-29T06:19:55Z

I signed it

mdc-web-bot · 2019-01-29T06:34:24Z

All 758 screenshot tests passed for commit b675f5c vs. feat/typescript! 💯🎉

kfranqueiro

I missed the boat prior to this being merged but I have a few questions and a couple of cleanup comments.

kfranqueiro · 2019-01-29T20:12:12Z

packages/mdc-ripple/util.ts

  if (typeof supportsCssVariables_ === 'boolean' && !forceRefresh) {
-    return supportsCssVariables;
+    return Boolean(supportsCssVariables_);


Does this really need this cast? The if is already confirming it's boolean...

TS Compiler was complaining without the case before. I think what I did was copy from rachel's CL and rewrote the logic.

kfranqueiro · 2019-01-29T20:13:51Z

packages/mdc-ripple/util.ts

  }

-  const explicitlySupportsCssVars = windowObj.CSS.supports('--css-vars', 'yes');
+  const {CSS} = windowObj;
+  const explicitlySupportsCssVars = CSS


The CSS && here and below seem redundant here because it's already checked above... is TS complaining without this?

kfranqueiro · 2019-01-29T20:16:18Z

packages/mdc-ripple/util.ts

  }

-  const explicitlySupportsCssVars = windowObj.CSS.supports('--css-vars', 'yes');
+  const {CSS} = windowObj;


Do the new typings files automatically get picked up by tsc with no configuration?

Correct if we don't define files or includes in the tsconfig.json file, TS defaults to finding all .d.ts files within the tsconfig's root directory.

https://www.typescriptlang.org/docs/handbook/tsconfig-json.

If the "files" and "include" are both left unspecified, the compiler defaults to including all TypeScript (.ts, .d.ts and .tsx) files in the containing directory and subdirectories except those excluded using the "exclude" property. JS files (.js and .jsx) are also included if allowJs is set to true. If the "files" or "include" properties are specified, the compiler will instead include the union of the files included by those two properties. Files in the directory specified using the "outDir" compiler option are excluded as long as "exclude" property is not specified.

kfranqueiro · 2019-01-29T20:29:02Z

packages/mdc-ripple/util.ts

- */
-function getMatchesProperty(HTMLElementPrototype) {
+/** Gets the matches function from an element. */
+export function getMatchesFunction(htmlElementPrototype: HTMLElement): (selector: string) => boolean {


Why does this function exist? It did not exist before, appears to be unused, and differs in behavior than getMatchesProperty (it incorrectly prefers vendor-prefixed versions, and it's incorrectly returning the webkit-prefixed version in the ms-prefixed branch).

Also something I picked up from Rachel's CL...I didn't check it for usages. Good catch

kfranqueiro · 2019-01-29T20:30:37Z

packages/mdc-ripple/util.ts

- */
-function getNormalizedEventCoords(ev, pageOffset, clientRect) {
+export function getNormalizedEventCoords(
+    ev: Event | undefined, pageOffset: Point, clientRect: ClientRect): Point {


Why are we allowing undefined here? It was not allowed to be undefined in the previous definition.

in foundation.ripple the activationEvent is defaulted to undefined. It then trickled to this function. Not sure what else to default the defaultActivationState.

kfranqueiro · 2019-01-29T21:03:37Z

packages/mdc-ripple/util.ts

-    ev = /** @type {!TouchEvent} */ (ev);
-    normalizedX = ev.changedTouches[0].pageX - documentX;
-    normalizedY = ev.changedTouches[0].pageY - documentY;
+    const e = ev as TouchEvent;


Can we give this variable a more descriptive name like touchEvent rather than an even more abbreviated name? Single-letter variables are notoriously harder to search usages of.

kfranqueiro · 2019-01-29T21:04:12Z

packages/mdc-ripple/util.ts

+    normalizedX = e.pageX - documentX;
+    normalizedY = e.pageY - documentY;
+  }
+  if (normalizedX === undefined || normalizedY === undefined) {


Is this actually possible to happen?

looks like that was added from an original PR to test typescript. So I think it is a relic from an older version of TS complaining. Checked with Rachel and it is not needed. Removing.

kfranqueiro · 2019-01-29T21:06:21Z

packages/mdc-ripple/index.ts

-  get unbounded() {
-    return this.unbounded_;
+  disabled = false;
+  root_: Element = this.root_;


Possibly dumb question: is the assignment here actually doing anything or is it just assigning this.root_ to itself?

it is just assigning it to itself. Without that line, this is not of type RippleCapableSurface. This causes line 116 to complain:

getDefaultFoundation(): MDCRippleFoundation { return new MDCRippleFoundation(MDCRipple.createAdapter(this)); }

Should I add a comment?

This reverts commit 47a9c38.

Matt Goo added 3 commits January 24, 2019 12:55

WIP: ripple

02799d4

Merge branch 'feat/typescript' into feat/checkbox-ripple-ts

7b26e5e

feat: ripple ts conversion

75cb9ba

googlebot added the cla: yes label Jan 25, 2019

fix: tests

97fed6f

mmalerba reviewed Jan 26, 2019

View reviewed changes

fix: ripple WIP

79542ed

fix: test

08f8c17

mmalerba reviewed Jan 28, 2019

View reviewed changes

Matt Goo added 4 commits January 28, 2019 11:32

fix: PR updates adapter readabiliy

18a6b15

fix: consolidate global types into mdc-dom

1d6db00

fix: remove extra backtick

ff619b5

feat: created global typings directory

78b9e4a

mmalerba reviewed Jan 29, 2019

View reviewed changes

moog16 commented Jan 29, 2019

View reviewed changes

packages/mdc-menu-surface/adapter.ts Outdated

@@ -0,0 +1,131 @@

/**

Copy link

Contributor Author

moog16 Jan 29, 2019

Choose a reason for hiding this comment

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

don't know how this ended up here...removing

fix: revert events.ts

7abb9d2

mmalerba reviewed Jan 29, 2019

View reviewed changes

mmalerba approved these changes Jan 29, 2019

View reviewed changes

fix: use SpecificEventListener

b675f5c

googlebot added cla: no and removed cla: yes labels Jan 29, 2019

moog16 merged commit 47a9c38 into feat/typescript Jan 29, 2019

moog16 deleted the feat/checkbox-ripple-ts branch January 29, 2019 06:55

kfranqueiro mentioned this pull request Jan 29, 2019

Convert packages to TypeScript #4225

Closed

45 tasks

kfranqueiro reviewed Jan 29, 2019

View reviewed changes

moog16 pushed a commit that referenced this pull request Jan 30, 2019

Revert "feat(ripple): convert to ts (#4300)"

516b0c3

This reverts commit 47a9c38.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ripple): convert to ts #4300

feat(ripple): convert to ts #4300

moog16 commented Jan 25, 2019

mdc-web-bot commented Jan 25, 2019

mdc-web-bot commented Jan 25, 2019

mmalerba Jan 25, 2019

mmalerba Jan 26, 2019

moog16 Jan 28, 2019

mdc-web-bot commented Jan 26, 2019

mmalerba Jan 28, 2019

moog16 Jan 28, 2019

moog16 Jan 28, 2019

mmalerba Jan 29, 2019

codecov-io commented Jan 29, 2019 •

edited

Loading

moog16 Jan 29, 2019

mdc-web-bot commented Jan 29, 2019

mmalerba Jan 29, 2019

googlebot commented Jan 29, 2019

moog16 commented Jan 29, 2019

mdc-web-bot commented Jan 29, 2019

kfranqueiro left a comment

kfranqueiro Jan 29, 2019

moog16 Jan 29, 2019 •

edited

Loading

kfranqueiro Jan 29, 2019

kfranqueiro Jan 29, 2019

moog16 Jan 29, 2019 •

edited

Loading

kfranqueiro Jan 29, 2019

moog16 Jan 29, 2019

kfranqueiro Jan 29, 2019

moog16 Jan 29, 2019

kfranqueiro Jan 29, 2019

kfranqueiro Jan 29, 2019

moog16 Jan 29, 2019

kfranqueiro Jan 29, 2019

moog16 Jan 29, 2019


		containsEventTarget(target: EventTarget \| null): boolean;

		registerInteractionHandler<K extends keyof GlobalEventHandlersEventMap>(


		deregisterDocumentInteractionHandler<K extends EventType>(evtType: K, handler: SpecificEventListener<K>): void;

		registerResizeHandler(handler: (evt: GlobalEventHandlersEventMap['resize']) => void): void;

feat(ripple): convert to ts #4300

feat(ripple): convert to ts #4300

Conversation

moog16 commented Jan 25, 2019

mdc-web-bot commented Jan 25, 2019

mdc-web-bot commented Jan 25, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdc-web-bot commented Jan 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jan 29, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

mdc-web-bot commented Jan 29, 2019

Choose a reason for hiding this comment

googlebot commented Jan 29, 2019

moog16 commented Jan 29, 2019

mdc-web-bot commented Jan 29, 2019

kfranqueiro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moog16 Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moog16 Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jan 29, 2019 •

edited

Loading

moog16 Jan 29, 2019 •

edited

Loading

moog16 Jan 29, 2019 •

edited

Loading