search

galtenberg / evernote-random

Use evernote API as a logged-in user - react and express project
Other
39 stars 6 forks source link

First commit: nicely styled, WIP #18

Closed callumflack closed 6 years ago

callumflack commented 6 years ago

1st draft, incomplete UI design. To note:

  1. The "save" instructions is a placeholder div for now. See it commented out at App/App.js:69

  2. The notebook filters list is also commented out for now. See Notes/Notebooks.js:91

  3. I use Prettier in VSCode. I frogot to turn it off before working, so it's done some reformatting… Should we decide on via a .prettierrc or should I turn it off?

  4. As discussed with main menu toggling filters / instructions, could you also please add an active class for the active button?

  5. This is my first go at OSS! I've prob not got this PR quite right. I'm all ears as we go thru this.

galtenberg commented 6 years ago

Great contribution, man - seriously -

I made a few specific prettier comments - it is always easier if things like that are done in separate PRs, so that we can easily see material differences. If those can be all the way backed out, we can chat this out later. See how much time it'd take either way (backing those changes out, vs. discussing specific points).

I'll have to look into how glitch plays with yarn files.

callumflack commented 6 years ago

Cheers Chris!

If yarn doesn't play with Glitch, can use npm, no fuss.

callumflack commented 6 years ago

Hi Chris, I made two fixes:

  1. Downgraded the node engine. Looks like it only bugs out when I need to yarn add deps. I can handle that manually as prob won't be adding much.

  2. Added a Prettier config for my editor. That way you don't need to add Prettier or anything. I also resaved each of the component files, reformatting them in the process.

Let me know if there's anything I've missed.

I'm aiming to work on this again on Friday afternoon.

galtenberg commented 6 years ago

Cool man - I'm actually testing package upgrades based on 10.x, I'll let you know how it goes.

And we'll need to stick on npm for glitch, but maybe we just gitignore both package-lock and yarn files, so we can work locally however's best.

callumflack commented 6 years ago

OK great. Too easy re glitch + npm.

galtenberg commented 6 years ago

Do add the prettier config, and I'll see if I can get my vim to auto apply it.

And/or maybe also add a linter that agrees with the prettier config?

callumflack commented 6 years ago

Re: eslint + prettier, here's a concise rundown of how CRA do it: https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md#formatting-code-automatically

I can then simply add our rules in the lint-staged object (single quotes / no semis).

Good for you?

I took the time to dig a little because eslint + prettier can get it messy pretty quickly. Turns out eslint is already w/i CRA, so I'm glad I checked. CRA handles it nicely.

galtenberg commented 6 years ago

@callumflack This sounds just right to me 👍 :beer:

callumflack commented 6 years ago

FYI: the node engines problem is easily countered by adding the --ignore-engines flag during installs

callumflack commented 6 years ago

@galtenberg OK, added that nice little lint-staged script. That's a much better way of handling formatting than managing 2 x config files for eslint & prettier. I put off figuring out the perfect solution for that for months.

Can you just check the globbing for me?