Frontend refactorings & dashboard features #53

berekuk · 2022-04-10T14:27:45Z

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.

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)

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

Dashboards:

since I touched on /dashboards code, doing UX changes to dashboards #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 · 2022-04-10T14:27:50Z

✅ Deploy Preview for metaforecast ready!

Name	Link
🔨 Latest commit	`6b0a9a4`
🔍 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 · 2022-04-11T17:13:51Z

src/web/display/DisplayForecast/ForecastFooter.tsx

+}) => {
+  // I experimented with justify-evenly, justify-around, etc., here: https://tailwindcss.com/docs/justify-content
+  // I came to the conclusion that as long as the description isn't justified too, aligning the footer symmetrically doesn't make sense
+  // because the contrast is jarring.


Not sure how I feel about this; for me, non-symmetric footers feel really jarring. But I'll defer to you.

Uh, that's your comment that I should've removed :)
I haven't changed anything re: footers alignment.

NunoSempere · 2022-04-11T17:16:09Z

docs/coding-style.md

+# React
+
+- create one file per one component (tiny helper components in the same file are fine)
+- if


Already fixed in 8d6cf48

NunoSempere · 2022-04-11T17:17:27Z

docs/coding-style.md

+# General notes
+
+- use `const` instead of `let` whenever possible
+- set up [prettier](https://prettier.io/) to format code on save


Thanks for the documenting!

NunoSempere · 2022-04-11T17:22:59Z

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 · 2022-04-11T17:38:57Z

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 · 2022-04-11T17:50:09Z

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 · 2022-04-11T17:54:01Z

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 · 2022-04-12T15:45:29Z

/secretEmbed is used by the twitter bot.

Is this PR ready to be merged?

vercel · 2022-04-12T19:21:45Z

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 · 2022-04-12T19:55:03Z

Is this PR ready to be merged?

Yes! ✔️

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

NunoSempere · 2022-04-12T20:19:58Z

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

NunoSempere · 2022-04-12T20:24:16Z

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

Nice

berekuk · 2022-04-12T20:29:00Z

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.

berekuk added 5 commits April 10, 2022 16:54

refactor: frontend code, especially DisplayForecast(s)

ce9bb0e

docs: coding style

d92f18d

feat: new dashboards logic, closes #17

986f7ab

feat: 404 on non-existent dashboards

115bd30

docs: typo cleanup

8d6cf48

fix: capitalize component files

62e3a37

NunoSempere reviewed Apr 11, 2022

View reviewed changes

feat: /dashboards/embed/[id] url schema

6b0a9a4

vercel bot deployed to Preview April 12, 2022 19:21 View deployment

berekuk merged commit e320d50 into master Apr 12, 2022

This was referenced Apr 12, 2022

Metaforecast was briefly down #57

Closed

Vercel and Terraform #55

Merged

berekuk deleted the forecast-html-css branch May 12, 2022 22:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Frontend refactorings & dashboard features #53

Frontend refactorings & dashboard features #53

berekuk commented Apr 10, 2022 •

edited

Loading

netlify bot commented Apr 10, 2022 •

edited

Loading

NunoSempere Apr 11, 2022

berekuk Apr 11, 2022

NunoSempere Apr 12, 2022

NunoSempere Apr 11, 2022

berekuk Apr 11, 2022

NunoSempere Apr 11, 2022

NunoSempere commented Apr 11, 2022

berekuk commented Apr 11, 2022

NunoSempere commented Apr 11, 2022 •

edited

Loading

berekuk commented Apr 11, 2022

NunoSempere commented Apr 12, 2022

vercel bot commented Apr 12, 2022 •

edited

Loading

berekuk commented Apr 12, 2022

NunoSempere commented Apr 12, 2022 •

edited

Loading

NunoSempere commented Apr 12, 2022 •

edited

Loading

berekuk commented Apr 12, 2022

Frontend refactorings & dashboard features #53

Frontend refactorings & dashboard features #53

Conversation

berekuk commented Apr 10, 2022 • edited Loading

netlify bot commented Apr 10, 2022 • edited Loading

✅ Deploy Preview for metaforecast ready!

NunoSempere Apr 11, 2022

Choose a reason for hiding this comment

berekuk Apr 11, 2022

Choose a reason for hiding this comment

NunoSempere Apr 12, 2022

Choose a reason for hiding this comment

NunoSempere Apr 11, 2022

Choose a reason for hiding this comment

berekuk Apr 11, 2022

Choose a reason for hiding this comment

NunoSempere Apr 11, 2022

Choose a reason for hiding this comment

NunoSempere commented Apr 11, 2022

berekuk commented Apr 11, 2022

NunoSempere commented Apr 11, 2022 • edited Loading

berekuk commented Apr 11, 2022

NunoSempere commented Apr 12, 2022

vercel bot commented Apr 12, 2022 • edited Loading

berekuk commented Apr 12, 2022

NunoSempere commented Apr 12, 2022 • edited Loading

NunoSempere commented Apr 12, 2022 • edited Loading

berekuk commented Apr 12, 2022

berekuk commented Apr 10, 2022 •

edited

Loading

netlify bot commented Apr 10, 2022 •

edited

Loading

NunoSempere commented Apr 11, 2022 •

edited

Loading

vercel bot commented Apr 12, 2022 •

edited

Loading

NunoSempere commented Apr 12, 2022 •

edited

Loading

NunoSempere commented Apr 12, 2022 •

edited

Loading