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

Feature/implement placeholder #6

Merged
merged 17 commits into from
Dec 20, 2020
Merged

Conversation

strausr
Copy link
Contributor

@strausr strausr commented Dec 16, 2020

PR for Placeholder!!

A few things to note-

  1. This PR is done off of accessibility which has yet to be merged
  2. Tests for combinations (i.e: placeholder+lazyload) is coming right up after this but was left out for clarity
  3. Some of the placeholder transformations are incomplete since there are missing from @base

Delivery.quality- there is a pr in place for it
blur() and pixelate() are throwing TS errors (i.e.Argument of type 'BlurAction' is not assignable to parameter of type 'EffectActions'.   Type 'BlurAction' is missing the following properties from type 'GradientFadeEffectAction': _type, type, horizontalStartPoint, verticalStartPoint) (will open a ticket for this)

  1. Predominant-color transformation was updated from v1 to be the size of the original image- we can't use a pixel anymore and this is also up to spec

@strausr strausr requested review from a user and patrick-tolosa December 16, 2020 15:22
html/src/constants.ts Outdated Show resolved Hide resolved
html/src/htmlLayer.ts Show resolved Hide resolved
html/src/placeholder.ts Outdated Show resolved Hide resolved
html/src/placeholder.ts Outdated Show resolved Hide resolved
html/src/placeholder.ts Outdated Show resolved Hide resolved
react/__tests__/placeholder.test.tsx Show resolved Hide resolved
@strausr strausr merged commit 5bce506 into main Dec 20, 2020
@strausr strausr deleted the feature/implement-placeholder branch December 29, 2020 07:08
strausr added a commit that referenced this pull request Nov 21, 2021
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.

2 participants