Frontend refactorings & dashboard features

berekuk commented 2 years ago

Ok, I got a bit carried away here (instead of working on graphql, going to start on that soon), but these are the cleanups I wanted to do eventually.

What's included:

Cleanups: coding style, extracted common components, enforced naming conventions, typescript. Also: initial docs/coding-style.md explaining what I consider to be best practice.
Simplified html/css.
Dashboards refactorings & new dashboard urls.

1) Cleanups:

all component files are now named according to their primary component, e.g. MyComponent.tsx
use named exports instead of default exports (it's easier not to mess rename refactorings this way, and also to type components properly)
most components are typed with React.FC<Props>
all components are rendered with JSX instead of function calls
significant overhaul of DisplayForecast component, split footer in a separate file (and should've split more stuff), removed some unused code, etc.
replaced most lets with consts wherever I've seen them :) (two more letters, but it's better for typing and clearer in intent)

2) Simplified HTML/CSS for DisplayForecast, /tools and /dashboards:

I probably removed >70% of css classes in the code I changed; some of them weren't doing anything useful, and some were overly complicated
Also extracted a few components, e.g. Card, Button and LastUpdated
New code doesn't render identically, but all changes are minor (unless I messed up some rare combination of showTimeStamp and screen size... but I think I checked most of it, e.g. how question cards look on /secretEmbed)
One aspect in which new code renders a bit differently is margins; it's generally a bad practice to mix margins with other code, so whenever I've encountered a few components with custom mt-N mb-M I tried to replace it with space-y-N in the parent component, and some sub-components shifted by a few pixels due to this

3) Dashboards:

since I touched on /dashboards code, doing #17 seems relevant
this lead me to url changes: dashboard pages are now at /dashboards/view/[id], with redirects from the old /dashboards?dashboardId=[id] (using middleware, see src/pages/_middleware.ts)
I could probably avoid the url change, but new schema seems better anyway (and the code was getting overly complicated with initialVariable and useEffects, I think trying to handle it the old way is too much of an anti-pattern, tbh)
also fixed some bugs, e.g. "Create dashboard" button rendering was broken during dashboard's creation, should be better now

netlify[bot] commented 2 years ago

Deploy Preview for metaforecast ready!

Name	Link
Latest commit	6b0a9a4c417124057d2eeeb3bf0f42096511cfd3
Latest deploy log	https://app.netlify.com/sites/metaforecast/deploys/6255d14798abb900081265e3
Deploy Preview	https://deploy-preview-53--metaforecast.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

NunoSempere commented 2 years ago

This looks good. Could you also create a dashboardEmbed with the same schema (dashboardEmbed/view/[id]), to replace secretDashboard, but without deleting the secretDashboard page yet?

berekuk commented 2 years ago

Could you also create a dashboardEmbed with the same schema (dashboardEmbed/view/[id]), to replace secretDashboard, but without deleting the secretDashboard page yet?

Maybe dashboard/embed/[id] for namespacing then? Or /dashboard/view/[id]/embed.

I could just add a redirect to it and keep /secretDashboard forever as legacy.

Btw, where is /secretDashboard used currently? I see by Netlify analytics that it gets a lot of traffic (from globalguessing.com, probably?), but I couldn't find any pages where it's embedded. Reason I'm asking: it dumps a raw json into html, and I wonder if all pages that embed it are currently messed up.

NunoSempere commented 2 years ago

Maybe dashboard/embed/[id] for namespacing then? Or /dashboard/view/[id]/embed.

dashboard/embed/[id] sounds a bit better.

I could just add a redirect to it and keep /secretDashboard forever as legacy.

Sounds good, though maybe not forever.

Btw, where is /secretDashboard used currently?

Here: https://globalguessing.com/russia-ukraine-forecasts/, that I know of

berekuk commented 2 years ago

Reason I'm asking: it dumps a raw json into html, and I wonder if all pages that embed it are currently messed up.

Oh, I mixed it up with /secretEmbed. /secretDashboard is fine.

NunoSempere commented 2 years ago

/secretEmbed is used by the twitter bot.

Is this PR ready to be merged?

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/quantified-uncertainty/metaforecast/5ZTfKLAW8aiqZH82jxMy1jssYKCC
✅ Preview: https://metaforecast-git-forecast-html-css-quantified-uncertainty.vercel.app

berekuk commented 2 years ago

Is this PR ready to be merged?

Yes! ✔️

Also, I added /dashboards/embed/[id] redirect in the final commit.

NunoSempere commented 2 years ago

I'm getting some errors in Vercel related to this commit @berekuk

temp

NunoSempere commented 2 years ago

Also, I added /dashboards/embed/[id] redirect in the final commit.

Nice

berekuk commented 2 years ago

I'm getting some errors in Vercel related to this commit

That's #15 again (due to me pushing longer versions of infer/gjopen cookies there). Fixed.

quantified-uncertainty / metaforecast