jmegner / KT21Calculator

Specialized calculator for analyzying shooting and fighting in Kill Team 2021 edition ("KT21").
https://jmegner.github.io/KT21Calculator/
The Unlicense
11 stars 4 forks source link

Changed Nav to be icons #2

Closed veddermatic closed 2 years ago

veddermatic commented 2 years ago

Made app not render calculator that was not being used Changed lodash imports to only bring in used functions Centered content in browser window Upgraded react-scripts mainly to get rid of webpack licence iframe Moved some dev dependencies down in package.json Added hillarious comment to commented out test

jmegner commented 2 years ago

Thanks for the pull request!

Changed Nav to be icons

Can we have hovertext "Shoot" and "Fight"? I'm addicted to hovertext for icons? Also, are the engage and conceal order icons just for possible future use?

Made app not render calculator that was not being used

Unfortunately that change also makes the shoot/fight section lose state when you swap to the other section. Ex: you are in shoot section, change MWx to 1, then go to fight section and back to shoot section, and you'll see that MWx is back to X (the default). I'd rather that users not lose work when toggling between shoot and fight sections.

Changed lodash imports to only bring in used functions

Good idea. Sidenote: you missed Util.ts, line 59, which uses _ and caused a syntax error for me.

Centered content in browser window

I like it.

Upgraded react-scripts mainly to get rid of webpack licence iframe

Could you walk me through how you did this? I think at one point I tried to do some upgrading of eslint and got so many issues I undid it. At another point, I did some other upgrade (react itself?) and then I couldn't use tsconfig.json's "baseUrl": "." to get import paths more absolute (within the project), so I undid that too. So, I have to ask if you did anything other than a npm update react-scripts?

Also, webpack license iframe, is that this thing?

Moved some dev dependencies down in package.json

Interesting. I can see why TypeScript and types packages are a dev dependency and not a runtime dependency, but now I'm confused why create-react-app would not put those things in devDependencies. Any insight?

jmegner commented 2 years ago

Oh, also, the icons shouldn't be in src/, right? maybe public/ or a subfolder of public?

veddermatic commented 2 years ago

Can we have hovertext "Shoot" and "Fight"? I'm addicted to hovertext for icons? Also, are the engage and conceal order icons just for possible future use?

Done

Good idea. Sidenote: you missed Util.ts, line 59, which uses _ and caused a syntax error for me.

Fixed.

Unfortunately that change also makes the shoot/fight section lose state when you swap to the other section

Oops, this is better now.

veddermatic commented 2 years ago

Images should be in src because they are imported, so they will be part of the build process. The certainly could live in their own directory, but should be below src/ wherever they wind up.

jmegner commented 2 years ago

Thanks again!