Port blog page to ts #2676

RoyEJohnson · 2024-10-23T21:24:44Z

bethshook · 2024-10-29T21:00:41Z

are there specific files you'd like eyes on? because this is pretty tricky to follow. in the future you can use git mv to explicitly rename the file and then the diffs will work rather than count the files as removed/added.

RoyEJohnson · 2024-10-29T22:16:22Z

src/app/pages/blog/blog.scss

@@ -5,6 +5,7 @@
    background-color: ui-color(white);
    justify-items: center;
    width: 100%;
+    overflow: clip;


This fixes a layout bug I noticed in the blog article: there is a sticky item that shows how much time is left in the article. It was not being sticky.

RoyEJohnson · 2024-10-29T22:23:47Z

src/app/components/toggle/toggle-control-bar.js

@@ -41,8 +42,11 @@ export default function ToggleControlBar({ Indicator, children }) {
            onClick={() => toggle()}
            onKeyDown={onKeyDown}
            ref={focusRef}
+            role="combobox"
+            aria-controls={listboxId}


This was an accessibility fix needed because the testing library identifies accessible objects and this wasn't one.

RoyEJohnson · 2024-10-29T22:28:07Z

@bethshook I have retrofitted file moves into the git history. The "Files changed" tab still shows them as delete and add (because the diffs are not minor enough to show as simply modified -- I moved some code blocks), but if you look in the individual commits after the "mv" or "rename" commits, you can see the file diffs. I think there are four such files:
src/app/pages/blog/article/article.js -> tsx
src/app/pages/blog/article/use-progress.js -> tsx
src/app/pages/blog/search-results/use-all-articles.js -> tsx
test/src/pages/blog/blog.test.js -> tsx

Additionally, blog-pages.tsx was extracted from blog.js (which needed to be js due to dynamic import).

Many test files are simply new. I'm sorry it's such a big PR. Changes are generally these types:

Adding or altering types for TS
Rearranging blocks so that the export default is at the top rather than the bottom
Some amount of reformatting by "prettier"
Tests and changes to test-data files (there's no point in reading through files in test/src/data) I'm hopeful the tests themselves are reasonably readable.

I'll make some comments in the code to explain things that I think might be confusing, and of course if you have questions about stuff, you can comment there.

RoyEJohnson · 2024-10-29T22:29:50Z

src/app/pages/blog/article-summary/article-summary.tsx

-    name?: string;
-    value: {collection: Collection}[];
-};
+type CollectionVariant =


These types were simply wrong. They shouldn't have been recursive, they just have an optional wrapping layer.

RoyEJohnson · 2024-10-29T22:33:30Z

src/app/pages/blog/article-summary/article-summary.tsx


-type BlurbData = null | {


BlurbData and ArticleSummaryData were defined independently, but BlurbData is really an extension of ArticleSummaryData.

RoyEJohnson · 2024-10-29T22:34:35Z

src/app/pages/blog/article/article.tsx

@@ -0,0 +1,267 @@
+import BodyUnit from '~/components/body-units/body-units';


Review as diffs here. Or maybe don't. The rearrangement of blocks is hard to follow because the order is basically reversed.
There are no functional changes. It's all about adding types.

RoyEJohnson · 2024-10-29T22:50:31Z

src/app/pages/blog/article/use-progress.tsx

@@ -0,0 +1,51 @@
+import React from 'react';


You can review as differences here. The big-block changes are just reformatting. Other than that, types.

RoyEJohnson · 2024-10-29T22:52:41Z

src/app/pages/blog/blog-context.tsx

@@ -0,0 +1,212 @@
+import React from 'react';


View as diffs here. For this one, it's just a bunch of types added. A big block of them at the top, and others peppered throughout.

RoyEJohnson · 2024-10-29T22:54:50Z

src/app/pages/blog/blog-pages.tsx

@@ -0,0 +1,106 @@
+import React, {useEffect} from 'react';


These routines were extracted from blog.js (the next file down). Types were added. If you had the two files in separate windows, it would be easy to see.

RoyEJohnson · 2024-10-29T22:59:11Z

src/app/pages/blog/search-results/use-all-articles.tsx

@@ -0,0 +1,33 @@
+import {useState, useEffect} from 'react';


As diffs here. The data.subheading change was due to subheading being a required field. Making the variable was just part of the figuring-it-out process; I should probably take that back out.

RoyEJohnson · 2024-10-29T23:00:32Z

test/src/data/article-page-data.js

@@ -0,0 +1,186 @@
+// Blog article data as returned by usePageData


These next three are just test-data files.

The article progress indicator is supposed to be sticky. The positioning got broken by some overflow on the page.

bethshook · 2024-11-05T02:28:54Z

src/app/pages/blog/article/article.tsx

+type BodyData = {
+    type: string;
+    value:
+        | string


This is a weird choice of prettier. 🤷

bethshook · 2024-11-05T02:33:57Z

test/src/pages/blog/search-results.test.tsx


 jest.mock('~/pages/blog/pinned-article/pinned-article', () => jest.fn());
 jest.mock('~/pages/blog/more-stories/more-stories', () => ({
    LatestBlurbs: jest.fn()
 }));
-jest.mock('~/pages/blog/search-results/use-all-articles', () => jest.fn());
+// jest.mock('~/pages/blog/search-results/use-all-articles', () => jest.fn());


bethshook · 2024-11-05T02:43:47Z

src/app/pages/blog/explore-page/explore-page.tsx

+function useSubjectSnippetForTopic(topic?: string) {
+    const {subjectSnippet} = useBlogContext();
+
+    console.info('***', {subjectSnippet, topic});


bethshook · 2024-11-05T02:46:38Z

src/app/pages/blog/gated-content-dialog/gated-content-dialog.tsx


 function SubjectSelector() {
-    const data = useDataFromSlug('snippets/subjects?locale=en');
+    const {subjectSnippet: data} = useBlogContext();
+    // const data = useDataFromSlug('snippets/subjects?locale=en');


bethshook

looks fine to me - just noted a few lines that look like they were added for dev purposes

RoyEJohnson requested a review from bethshook October 23, 2024 21:41

RoyEJohnson force-pushed the port-blog-page-to-ts branch 2 times, most recently from 013cfc5 to 49e8101 Compare October 29, 2024 21:57

RoyEJohnson commented Oct 29, 2024

View reviewed changes

RoyEJohnson added 12 commits November 4, 2024 09:32

port blog-context (coverage incomplete)

dbc10cf

mv article and use-progress

f3325e9

Update article and use-progress

f90d8e6

pinned-article

f0b87af

explore-page

fc2c0a9

gated-content-dialog (wip)

9423e41

latest-blog-posts and more-stories

a37ae83

mv use-all-articles

cc31ca7

Update use-all-articles

f85725e

rename blog test

a6ef32a

blog-pages

e8ef08e

Fix sticky bit not being sticky

3deed37

The article progress indicator is supposed to be sticky. The positioning got broken by some overflow on the page.

RoyEJohnson force-pushed the port-blog-page-to-ts branch from 49e8101 to 3deed37 Compare November 4, 2024 15:32

bethshook reviewed Nov 5, 2024

View reviewed changes

bethshook approved these changes Nov 5, 2024

View reviewed changes

Clean up leftovers (code review)

fb5d7bd

RoyEJohnson merged commit c875d12 into main Nov 6, 2024
1 check passed

RoyEJohnson deleted the port-blog-page-to-ts branch November 6, 2024 16:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port blog page to ts #2676

Port blog page to ts #2676

RoyEJohnson commented Oct 23, 2024 •

edited by jira bot

Loading

bethshook commented Oct 29, 2024

RoyEJohnson Oct 29, 2024

RoyEJohnson Oct 29, 2024

RoyEJohnson commented Oct 29, 2024

RoyEJohnson Oct 29, 2024

RoyEJohnson Oct 29, 2024

RoyEJohnson Oct 29, 2024 •

edited

Loading

RoyEJohnson Oct 29, 2024

RoyEJohnson Oct 29, 2024

RoyEJohnson Oct 29, 2024

RoyEJohnson Oct 29, 2024 •

edited

Loading

RoyEJohnson Oct 29, 2024

bethshook Nov 5, 2024

RoyEJohnson Nov 6, 2024

bethshook Nov 5, 2024

bethshook Nov 5, 2024

bethshook Nov 5, 2024

bethshook left a comment

		@@ -0,0 +1,267 @@
		import BodyUnit from '~/components/body-units/body-units';

		@@ -0,0 +1,186 @@
		// Blog article data as returned by usePageData

Port blog page to ts #2676

Port blog page to ts #2676

Conversation

RoyEJohnson commented Oct 23, 2024 • edited by jira bot Loading

bethshook commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyEJohnson commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyEJohnson Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyEJohnson Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bethshook left a comment

Choose a reason for hiding this comment

RoyEJohnson commented Oct 23, 2024 •

edited by jira bot

Loading

RoyEJohnson Oct 29, 2024 •

edited

Loading

RoyEJohnson Oct 29, 2024 •

edited

Loading