Skip to content

Feature/react styleguidist#5

Open
Anzumana wants to merge 6 commits into
JaKaTaK:gh-pagesfrom
Anzumana:feature/react-styleguidist
Open

Feature/react styleguidist#5
Anzumana wants to merge 6 commits into
JaKaTaK:gh-pagesfrom
Anzumana:feature/react-styleguidist

Conversation

@Anzumana

Copy link
Copy Markdown

Should be great to see what components we have to make contributing even easier.

Comment thread react-app/src/App.js
this.down = this.down.bind(this);
this.up = this.up.bind(this);
this.keyCodeMaps = {
27: 'ESC',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The quoting here looks odd to me but probably just work; though, I'd say is definitely something stylistic to consider.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

do you mean you would like " " better ?
i usually use for stings ' ' just because i like to do " " for the values inside the html/ jsx
But like you said both would work.
I think i will look into what formatter we could add to the pre commit. So that at least everything is consisted throughout the project

@AelMalinka AelMalinka Nov 1, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry it didn't show what I was meaning very well but if you look down a bit lines (+22 to +31) suddenly the keys and values are both double quoted and then further down it goes back to the keys being unquoted and the values being single quoted. I know JS likes to be a bit loose with typing but I wanted to make sure I pointed it out in case it could cause issues.

As a few side notes. I usually stick to single quotes in JS and as for a style checker I recently used https://eslint.org/ and wasn't horribly disappointed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok i just changed the strings so that we use ' when possible.
The only strings that will always be "incorrect"
will be '"' and "'"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i will look into adding eslint

@Troto

Troto commented Oct 31, 2019

Copy link
Copy Markdown

I don't really like adding only adrenal glads to the icons file to make that one example work. It feels kinda tacked on to what is otherwise just adding styleguidist?

@Anzumana

Anzumana commented Nov 1, 2019

Copy link
Copy Markdown
Author

yeah i was thinking about only adding styleguidist. but to make sure that there are not issues i added the small sample.
On a seperate branch i am a bit further with the icons component.
Currently looking to what type of styling would work the best

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.

3 participants