Add typescript/#42 #47

talesdsp · 2020-04-12T01:12:01Z

No description provided.

talesdsp · 2020-04-12T01:14:34Z

Obs: I found that sometimes running tests with dist folder created throws an Emit skip error. Just warning in case it happens with you.

talesdsp · 2020-04-12T01:16:47Z

src/Block.tsx

+      shadowRadius: elevation
+    },
+    space && { justifyContent: `space-${space}` },
+    card && { borderRadius: 12 },


Apparently SIZES.border is not declared, and I don't know what the value should be

We need to add a new value for SIZES.borderRadius

talesdsp · 2020-04-12T01:19:52Z

src/theme.ts

+  info: "#4DA1FF"
+};
+
+export const SIZES: Sizes = {


The SIZES constant. It should be radius?

SIZES is an object with values for different usage.
For example: radius is used for borderRadius

Yes! I should have been clear than that. What I mean is, the SIZES.radius is the correct value for the failing test below?

talesdsp · 2020-04-12T01:20:23Z

src/global.ts

+  info: string;
+};
+
+export type Sizes = {


The SIZES type

talesdsp · 2020-04-12T01:27:37Z

src/__tests__/Block.test.tsx

@@ -421,7 +421,8 @@ describe("<Block />", () => {

    const style = StyleSheet.flatten(component.props.style);
    expect(instance.props.card).toEqual(true);
-    expect(style.borderRadius).toEqual(SIZES.border);
+    //!TODO: border value
+    expect(style.borderRadius).toEqual(SIZES.height);
  });


Needs fix too

hetmann

I notice that the latest changes for #33 where not included in your PR.

hetmann · 2020-04-12T07:03:47Z

src/Button.tsx

  const {
    disabled,
    opacity,
    outlined,
    flex,
    height,
-    borderWidth,


The last commit had the fix for borderWidth & borderColor

hetmann · 2020-04-12T07:05:07Z

src/Button.tsx

  if (disabled) {
+    const backgroundColor = StyleSheet.flatten(buttonStyles).backgroundColor;


This can be extracted outside the if condition as it is used multiple times. See the last PR merge

hetmann · 2020-04-13T19:14:25Z

@taleco22 I would like to add ONLY the type definitions for components as requested in #42
This will keep the library as light as possible, using a .d.ts file for the component/s.

…-typescript/react-ui-kit#42

talesdsp · 2020-04-22T07:18:48Z

I 've fixed what was missing from #33. Extracted the background. Removed the .tsx files. And kept just the type definitions now 😅👍

Is there anything else?

hetmann · 2020-04-22T09:28:22Z

src/Button.js

@@ -205,7 +209,7 @@ function Button(props) {
    buttonStyles.backgroundColor = "transparent";
  }

-  const ButtonType = highlight
+  const touch = highlight


touch can be named Touchable or ButtonInstance and instead of RenderButton you can call it like this:

<Touchable {...extraProps} disabled={disabled} activeOpacity={opacity} style={buttonStyles}> {children} </Touchable>

hetmann · 2020-04-22T09:29:28Z

I 've fixed what was missing from #33. Extracted the background. Removed the .tsx files. And kept just the type definitions now 😅👍

Is there anything else?

@taleco22 Yes, I reviewed the code and I have left a comment on the Button component

msanvarov · 2020-05-08T02:47:28Z

Hey @hetmann, love the work that is being done with this library, but I plead that you allow @talesdsp to continue with Typescript development. In my attempts to extend and even use this component library, I have come across some issues that can be solved if this library is ported back to Typescript. I'm willing to contribute along with @talesdsp to get it back to the state it was in before. I think declaration files are not enough for this type of ui library. Here is an example of another popular react native library built with Typescript: https://github.com/akveo/react-native-ui-kitten.

talesdsp · 2020-05-08T08:46:54Z

@msanvarov
Issues with usage? Can you be more specific?
Well, if the project turns out to typescript again mostly is already done up here. Won't be a pain to re-add it.

hetmann · 2020-05-08T12:58:51Z

@talesdsp @msanvarov guys you can create a new branch named "ts" or "typescript" and use that in your projects, would that be helpful?

msanvarov · 2020-05-08T13:31:51Z

@msanvarov
Issues with usage? Can you be more specific?
Well, if the project turns out to typescript again mostly is already done up here. Won't be a pain to re-add it.

To begin with the utility helpers don't have types, so using them means I have to copy the test cases to understand the implementation. In terms of the implementation, stuff like this const isArray = typeof padding === "object"; seems weird because to check if a variable is an array, the following can be used Array.isArray(padding)

I also don't understand why this is done:

export interface CardProps extends CardOptions, ThemeProps {
  [key: string]: any;
}

This defeats the purpose of typing if you are going to allow any key value to be accepted as a prop.

hetmann · 2020-05-08T15:54:04Z

@msanvarov
Issues with usage? Can you be more specific?
Well, if the project turns out to typescript again mostly is already done up here. Won't be a pain to re-add it.

To begin with the utility helpers don't have types, so using them means I have to copy the test cases to understand the implementation. In terms of the implementation, stuff like this const isArray = typeof padding === "object"; seems weird because to check if a variable is an array, the following can be used Array.isArray(padding)

I also don't understand why this is done:
export interface CardProps extends CardOptions, ThemeProps {
  [key: string]: any;
}
This defeats the purpose of typing if you are going to allow any key value to be accepted as a prop.

I was using the old style "typeof" checking for types :)
To understand the utils you can read the comments, I did my best to add an easy to understand usage.

msanvarov · 2020-05-08T15:56:58Z

@msanvarov
Issues with usage? Can you be more specific?
Well, if the project turns out to typescript again mostly is already done up here. Won't be a pain to re-add it.

To begin with the utility helpers don't have types, so using them means I have to copy the test cases to understand the implementation. In terms of the implementation, stuff like this const isArray = typeof padding === "object"; seems weird because to check if a variable is an array, the following can be used Array.isArray(padding)
I also don't understand why this is done:
export interface CardProps extends CardOptions, ThemeProps {
  [key: string]: any;
}
This defeats the purpose of typing if you are going to allow any key value to be accepted as a prop.
I was using the old style "typeof" checking for types :)
To understand the utils you can read the comments, I did my best to add an easy to understand usage.

I think it will be cool is get jsdoc integration for the examples you made then when we create a gitbook to detail using this library, it will be easier to showcase some use cases for it.

hetmann · 2020-05-09T17:25:51Z

@msanvarov
Issues with usage? Can you be more specific?
Well, if the project turns out to typescript again mostly is already done up here. Won't be a pain to re-add it.

To begin with the utility helpers don't have types, so using them means I have to copy the test cases to understand the implementation. In terms of the implementation, stuff like this const isArray = typeof padding === "object"; seems weird because to check if a variable is an array, the following can be used Array.isArray(padding)
I also don't understand why this is done:
export interface CardProps extends CardOptions, ThemeProps {
  [key: string]: any;
}
This defeats the purpose of typing if you are going to allow any key value to be accepted as a prop.
I was using the old style "typeof" checking for types :)
To understand the utils you can read the comments, I did my best to add an easy to understand usage.
I think it will be cool is get jsdoc integration for the examples you made then when we create a gitbook to detail using this library, it will be easier to showcase some use cases for it.

I have in plan to do an actual app for this UI kit :) see #23
Also the jsdoc or gitbook would be nice to have

talesdsp added 7 commits April 11, 2020 03:54

add deps + command

2958ca1

add directory

f90e21d

setup typescript

35e1a0d

remove js files

a34fa40

add types for components

89e76d4

move typings to types folder

7652ed5

refactor

f449d6a

talesdsp commented Apr 12, 2020

View reviewed changes

hetmann requested changes Apr 12, 2020

View reviewed changes

talesdsp added 7 commits April 22, 2020 03:04

especify files in ts config

4a4c605

add scripts and filter npm files

1d562ab

delete files

aec3730

add components types

8ce390e

move interfaces

c3a132f

add type definition for components

302b9f2

Merge branch 'master' of github.com:react-ui-kit/expo-ui-kit into add…

c34c9bf

…-typescript/react-ui-kit#42

hetmann requested changes Apr 22, 2020

View reviewed changes

naming instance

6092e49

hetmann merged commit 7900837 into react-ui-kit:master Apr 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typescript/#42 #47

Add typescript/#42 #47

talesdsp commented Apr 12, 2020

talesdsp commented Apr 12, 2020

talesdsp Apr 12, 2020

hetmann Apr 12, 2020

talesdsp Apr 12, 2020

hetmann Apr 12, 2020

talesdsp Apr 12, 2020

talesdsp Apr 12, 2020

talesdsp Apr 12, 2020

talesdsp Apr 12, 2020

hetmann left a comment

hetmann Apr 12, 2020

hetmann Apr 12, 2020

hetmann commented Apr 13, 2020

talesdsp commented Apr 22, 2020 •

edited

Loading

hetmann Apr 22, 2020

hetmann commented Apr 22, 2020 •

edited

Loading

msanvarov commented May 8, 2020

talesdsp commented May 8, 2020 •

edited

Loading

hetmann commented May 8, 2020

msanvarov commented May 8, 2020 •

edited

Loading

hetmann commented May 8, 2020

msanvarov commented May 8, 2020

hetmann commented May 9, 2020 •

edited

Loading

		if (disabled) {
		const backgroundColor = StyleSheet.flatten(buttonStyles).backgroundColor;

Add typescript/#42 #47

Add typescript/#42 #47

Conversation

talesdsp commented Apr 12, 2020

talesdsp commented Apr 12, 2020

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

hetmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hetmann commented Apr 13, 2020

talesdsp commented Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

hetmann commented Apr 22, 2020 • edited Loading

msanvarov commented May 8, 2020

talesdsp commented May 8, 2020 • edited Loading

hetmann commented May 8, 2020

msanvarov commented May 8, 2020 • edited Loading

hetmann commented May 8, 2020

msanvarov commented May 8, 2020

hetmann commented May 9, 2020 • edited Loading

talesdsp commented Apr 22, 2020 •

edited

Loading

hetmann commented Apr 22, 2020 •

edited

Loading

talesdsp commented May 8, 2020 •

edited

Loading

msanvarov commented May 8, 2020 •

edited

Loading

hetmann commented May 9, 2020 •

edited

Loading