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.

Sign up for GitHub

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

Jump to bottom

feat(pie-thumbnail): DSW-2772 add default placeholder #2195

Merged
merged 25 commits into from
Feb 12, 2025
Merged

feat(pie-thumbnail): DSW-2772 add default placeholder #2195

merged 25 commits into from
Feb 12, 2025

Conversation

raoufswe
Copy link
Member

@raoufswe raoufswe commented Feb 6, 2025 •
edited by ashleynolan
Loading

Describe your changes (can list changeset entries if preferable)

  • Adds default placeholder on image load failures.

Author Checklist (complete before requesting a review, do not delete any)

  • I have performed a self-review of my code.
  • I have added thorough tests where applicable (unit / component / visual).
  • I have reviewed the PIE Storybook/PIE Docs PR preview.
  • I have reviewed visual test updates properly before approving.
  • If changes will affect consumers of the package, I have created a changeset entry.
  • If a changeset file has been created, I have tested these changes in PIE Aperture using the /test-aperture command.

Not-applicable Checklist items

Please move any Author checklist items that do not apply to this pull request here.

Testing

How do I test my changes?

Task Link
Aperture PR 🔗
NextJS 14 deployment 🔗
Nuxt 3 deployment 🔗
Vanilla deployment 🔗

Reviewer checklists (complete before approving)

Mark items as [-] N/A if not applicable.

Reviewer 1 - @fernandofranca

  • I have reviewed the PIE Storybook/PIE Docs PR preview.
  • I have verified that all acceptance criteria for this ticket have been completed.
  • [-] I have reviewed the Aperture changes (if added)
  • If there are visual test updates, I have reviewed them.

Reviewer 2

  • I have reviewed the PIE Storybook/PIE Docs PR preview.
  • I have verified that all acceptance criteria for this ticket have been completed.
  • I have reviewed the Aperture changes (if added)
  • If there are visual test updates, I have reviewed them.

@raoufswe raoufswe requested review from a team as code owners February 6, 2025 16:19
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 6, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: edc4de5

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@raoufswe raoufswe force-pushed the dsw-2772-default-placeholder branch from 05a8708 to 293add3 Compare February 6, 2025 16:23
@github-actions github-actions bot added tools and removed tools labels Feb 6, 2025
@raoufswe
fix(pie-thumbnail): DSW-2772 add default placeholder
50e5f15 
feat(pie-thumbnail): DSW-2583 add hasPadding thumbnail prop

feat(pie-thumbnail): DSW-2582 add size prop

feat(pie-thumbnail): DSW-2582 add size prop

fix(pie-thumbnail): DSW-2582 revert some changes

fix(pie-thumbnail): DSW-2582 revert some changes

fix(pie-thumbnail): DSW-2582 add sizes

fix(pie-thumbnail): DSW-2615 apply background to the image to respect the asepct ratio

fix(pie-thumbnail): DSW-2615 rever some changes

fix(pie-thumbnail): DSW-2615 revert some changes

fix(pie-thumbnail): DSW-2615 update stories

fix(pie-thumbnail): DSW-2615 update stories

fix(pie-thumbnail): DSW-2615 fix tests

fix(pie-thumbnail): DSW-2615 fix tests

fix(pie-monorepo): DSW-000 revert changes

fix(pie-thumbnail): DSW-2772 add default placeholder

fix(pie-thumbnail): DSW-2772 update styles

fix(pie-thumbnail): DSW-2772 revert some changes

fix(pie-thumbnail): DSW-2772 revert some changes
@raoufswe raoufswe force-pushed the dsw-2772-default-placeholder branch from 287087e to 50e5f15 Compare February 6, 2025 16:30
@raoufswe raoufswe changed the title Dsw 2772 default placeholder feat(pie-thumbnail): DSW-2772 add default placeholder Feb 6, 2025
raoufswe added 16 commits February 6, 2025 23:03
@raoufswe
fix(pie-thumbnail): DSW-2772 add tests
04c9b5e
@raoufswe
fix(pie-thumbnail): DSW-2772 update tests
08b6183
@raoufswe
fix(pie-thumbnail): DSW-2772 update tests
fe511b8
@raoufswe
fix(pie-thumbnail): DSW-2772 update svg and bundlewatch
007ce37
@raoufswe
fix(pie-thumbnail): DSW-2772 update tests
993b395
@raoufswe
fix(pie-thumbnail): DSW-2772 update jsdoc
b3b597f
@raoufswe
fix(pie-thumbnail): DSW-2772 an attempt to simplify the fallback logic
3ba5b7e
@raoufswe
fix(pie-thumbnail): DSW-2772 an attempt to simplify the fallback logic
94cda60
@raoufswe
fix(pie-thumbnail): DSW-2772 revert some changes
c5bb8f1
@raoufswe
fix(pie-thumbnail): DSW-2772 update logic
6d8660a
@raoufswe
fix(pie-thumbnail): DSW-2772 update logic
a1ecae1
@raoufswe
Merge branch 'main' into dsw-2772-default-placeholder
0d75c1f
@raoufswe
fix(pie-thumbnail): DSW-2772 update logic
3608819
@raoufswe
fix(pie-thumbnail): DSW-2772 add changeset
90dabbe
@raoufswe
fix(pie-thumbnail): DSW-2772 increase bundle size
fe22ad2
@raoufswe
fix(pie-thumbnail): DSW-2772 fix tests
003af99
@raoufswe raoufswe self-assigned this Feb 7, 2025
fernandofranca
fernandofranca previously approved these changes Feb 10, 2025
View reviewed changes
ashleynolan
ashleynolan previously approved these changes Feb 10, 2025
View reviewed changes
@ashleynolan
Copy link
Contributor

ashleynolan commented Feb 10, 2025 •
edited
Loading

From testing in storybook – should bgColor and aspectRatio affect the thumbnail when the placeholder is set as default? (I'm guessing it should, but it wasn't changing these when I was changing the controls)

@raoufswe raoufswe dismissed stale reviews from ashleynolan and fernandofranca via e07b3ee February 10, 2025 16:19
@siggerzz
Copy link
Contributor

/snapit

@pie-design-system-bot
Copy link
Contributor

Starting a new snapshot build. You can view the logs here.

@pie-design-system-bot
Copy link
Contributor

@siggerzz Your snapshots have been published to npm!

Test the snapshots by updating your package.json with the newly-published versions:

Note

If you have more than one of these packages installed, we suggest using the new snapshots for all of them to help avoid version conflicts.

yarn up @justeattakeaway/[email protected] --mode=update-lockfile
yarn up @justeattakeaway/[email protected] --mode=update-lockfile

Then finally:

yarn install

@siggerzz
Copy link
Contributor

/test-aperture

@pie-design-system-bot
Copy link
Contributor

Starting a new snapshot build. You can view the logs here.

@pie-design-system-bot
Copy link
Contributor

@siggerzz Your snapshots have been published to npm!

Test the snapshots by updating your package.json with the newly-published versions:

Note

If you have more than one of these packages installed, we suggest using the new snapshots for all of them to help avoid version conflicts.

yarn up @justeattakeaway/[email protected] --mode=update-lockfile
yarn up @justeattakeaway/[email protected] --mode=update-lockfile

Then finally:

yarn install

@raoufswe raoufswe requested a review from ashleynolan February 11, 2025 14:19
@raoufswe raoufswe merged commit df8ddf7 into main Feb 12, 2025
34 of 36 checks passed
@raoufswe raoufswe deleted the dsw-2772-default-placeholder branch February 12, 2025 09:49
@pie-design-system-bot pie-design-system-bot mentioned this pull request Feb 12, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fernandofranca fernandofranca fernandofranca approved these changes

@ashleynolan ashleynolan ashleynolan approved these changes

Assignees

@raoufswe raoufswe

Labels
apps components pie-storybook pie-thumbnail
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@raoufswe @pie-design-system-bot @ashleynolan @siggerzz @fernandofranca