Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Triptych app changes #2038

Merged
merged 33 commits into from
May 10, 2024
Merged

WIP: Triptych app changes #2038

merged 33 commits into from
May 10, 2024

Conversation

cmaddox5
Copy link
Contributor

@cmaddox5 cmaddox5 commented Apr 30, 2024

Asana task: [Timebox] Investigate babel config to possibly transpile from react to plain JS

Working on some webpack changes for getting the packaged triptych app compiled down to something the hardware can handle. Will update PR as OFM gives app feedback.

App has been running well on OFM lab screens so this may be the best we can do at the moment. Some key changes:

  • A new webpack entry has been created for Triptych packaging. This eliminates the need to change webpack when we are packaging up the app. The babel config for the new entry targets the oldest browsers possible (this is done by default when no targets are provided).

  • All images in Triptych widgets are imported by default so that webpack preloads them in specific folders. We then only zip those folders up in the OFM package. This helps keep package size down.

  • Any changes made to the existing webpack entry are there to allow backwards compatibility. This should only be a file-loader config for images. I found that if one entry configured file-loader for images, others needed to as well. Regardless, it seemed like a good idea to explicitly configure them.

  • Tests added?

Copy link

github-actions bot commented May 1, 2024

Coverage of commit d1c558a

Summary coverage rate:
  lines......: 44.5% (2846 of 6395 lines)
  functions..: 42.0% (1019 of 2428 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@cmaddox5 cmaddox5 marked this pull request as ready for review May 6, 2024 15:12
Copy link

github-actions bot commented May 6, 2024

Coverage of commit a2839ce

Summary coverage rate:
  lines......: 44.5% (2846 of 6395 lines)
  functions..: 42.0% (1019 of 2428 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

assets/index.d.ts Outdated Show resolved Hide resolved
assets/src/components/v2/outfront_evergreen_content.tsx Outdated Show resolved Hide resolved
assets/src/components/v2/triptych/no_data.tsx Outdated Show resolved Hide resolved
assets/src/hooks/v2/use_api_response.tsx Show resolved Hide resolved
@@ -25,7 +25,10 @@ export const formatTimeString = (timeString: string) =>
moment(timeString).tz("America/New_York").format("h:mm");

export const imagePath = (fileName: string): string =>
isOFM() ? `images/${fileName}` : `/images/${fileName}`;
isOFM() ? outfrontImagePath(fileName) : `/images/${fileName}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're still using imagePath for OFMs in addition to importing images into components, in which scenarios do we use which one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import should only have to be done once in outfront_evergreen_content.tsx. It gives webpack the ability to preload the images and save them to the correct folder. Because this folder only exists in the packaged app, we only need to reference that path on live triptychs. Eventually, DUPs should be treated similarly, but I wanted to leave them alone for now.

assets/webpack.config.js Outdated Show resolved Hide resolved
docs/triptych-react-app-testing.md Outdated Show resolved Hide resolved
Copy link

github-actions bot commented May 7, 2024

Coverage of commit e055290

Summary coverage rate:
  lines......: 44.5% (2846 of 6395 lines)
  functions..: 42.0% (1019 of 2428 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link
Contributor

@digitalcora digitalcora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tentative thumbs-up, let's see if it works on the real thing!

@digitalcora
Copy link
Contributor

Oh, it looks like this has some conflicts, since unfortunately it branched off before my linting changes 😓 Sorry for the extra work!

Copy link

github-actions bot commented May 9, 2024

Coverage of commit 5cec306

Summary coverage rate:
  lines......: 44.5% (2846 of 6395 lines)
  functions..: 42.0% (1019 of 2428 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@cmaddox5 cmaddox5 merged commit ff306a1 into main May 10, 2024
11 checks passed
@cmaddox5 cmaddox5 deleted the jz/resurrect-triptych-react-app branch May 10, 2024 17:16
@digitalcora
Copy link
Contributor

ah... "WIP" commit on main 😅

@cmaddox5
Copy link
Contributor Author

ah... "WIP" commit on main 😅

Dangit. I even looked right at those letters and thought "Yeah looks good"...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants