stuyspec.com icon indicating copy to clipboard operation
stuyspec.com copied to clipboard

Winter crossword

Open alychoi opened this issue 5 years ago • 4 comments

alychoi avatar Feb 19 '21 23:02 alychoi

Hey original author here. I'm a little curious about your decision to include node-sass and styled-components, as the code already uses react-jss for styles. I'd strongly recommend not using another styling library as the bundle size is already pretty inflated. Ditto with semantic-ui.

NicholasLYang avatar Mar 02 '21 18:03 NicholasLYang

Hey Nicholas,

We used a crossword component from an outside source from here https://www.npmjs.com/package/@jaredreisinger/react-crossword because we thought this would be much easier than making a crossword ourselves from scratch. If we can reduce the bundle size, would you still not recommend using this styling library? If not, we can alter it.

Also, thank you for your thorough feedback and suggestions in the other email. We all appreciate it!

On Tue, Mar 2, 2021 at 1:30 PM Nicholas Yang [email protected] wrote:

Hey original author here. I'm a little curious about your decision to include node-sass and styled-components, as the code already uses react-jss for styles. I'd strongly recommend not using another styling library as the bundle size is already pretty inflated. Ditto with semantic-ui.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stuyspec/stuyspec.com/pull/305#issuecomment-789119131, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACG56YK4UMIF5VVKIRVDH6TTBUVEHANCNFSM4X5FQTJA .

alychoi avatar Mar 02 '21 20:03 alychoi

I believe @DJankauskas has some experience making crosswords. Perhaps he can advise. I'd still avoid pulling in styled-components and using react-jss just for general cohesiveness of the code

NicholasLYang avatar Mar 02 '21 22:03 NicholasLYang

Frankly I also pulled in a library when I built this crossword site, although I used a Svelte library that seems like a version of the one the NYT uses. I don't disagree with deciding to use an external library, but I do think you should consider the implications of shipping more code with stuyspec.com. For that reason, I recommend you lazy-load either react-crossword, or ideally, most of the crossword page using code splitting. It's pretty easy to do, and would mean that users only need to download the code necessary when they check out a crossword. I agree with @NicholasLYang that you should aim to use react-jss instead of styled-components.

Two more recommendations I'd add: first, try converting your code here to use Typescript. It really helps making programming in React far less buggy (like making sure you handle values that might be null or undefined.) Second, consider writing a generic CrosswordPage that takes info like the title and clues as props rather than hardcoding a new component for each puzzle. That'll reduce the code you copy and paste and keep code size down.

DJankauskas avatar Mar 02 '21 23:03 DJankauskas