e2nguyen / Watcher

watcher-blond.vercel.app
2 stars 2 forks source link

[MVP]: Custom Image Component #23

Closed e2nguyen closed 3 years ago

e2nguyen commented 3 years ago

Description

Our project relies heavily on the use of images with custom features, so we'll need to create a custom image component with the following: nextjs's image component, tracking.

New: Out of scope, but added components playground refactor

Branches

Requires

Potential solutions

Including Images

Option 1

  1. Tried to create a custom images folder to store images.
  2. Ran into issues with importing images via absolute path in the import block. Experienced loader issues with babel.

Option 2

  1. Alternative here, would be to define*png as a custom .d.ts type.
  2. Then, load these images into said .tsx file via a require.
  3. This is silly and bad practice.

Option 3: SOLUTION

  1. Rely on React's method of importing images by capitalizing on the public folder
  2. This way, images can be included by their absolute path starting from the public folder.
  3. No need for any form of explicit imports here.

Adding layout prop

Option 1

  1. Tried to declare layout: string in the Prop interface.
  2. Failed because string cannot be assigned to "fixed" | "responsive" | "intrinsic" | undefined
    • this is saying string is a superset of "fixed" | "responsive" | "intrinsic" | undefined, so it can't be implicitly converted to a more specific type
    • it's like a dog is an animal, but and animal is not a dog. An animal is a broader term that includes a cat, bird, etc

Option 2: SOLUTION

  1. casted layout as layout?: "fixed" | "responsive" | "intrinsic"
  2. this is good because it forces to user to supply one of the following
  3. Additionally, realized layout doesn't necessarily always need to be supplied on declaration, so made it optional with ? syntax
  4. added a default prop for the layout prop to be complete

Tasks

QA Notes

Deployment Notes

PR

e2nguyen commented 3 years ago

@dianephan, can you review this? thanks

dianephan commented 3 years ago

done reviewing! did a quick visual QA by pulling this branch and testing it out on my end. code is clean and looks ready to merge