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

Responsive Image Component #992

Closed
wants to merge 2 commits into from
Closed

Conversation

AyushRathi07
Copy link

My team followed TSP and created a new component to display responsive images using srcset. Now, the developers using this design system can use this component to give multiple URLs of images that need to be displayed depending on the viewport size.

Also, the 'rush create' command is missing from your scripts, so we were not able to follow the procedure mentioned in the document but we have thoroughly tested our component for any errors and corrected if found any.

@craigpalermo
Copy link
Collaborator

Hey @AyushRathi07, thanks for contributing! Many of us are out of office for the holidays, but we'll review this the next time our working group meets.

@craigpalermo craigpalermo added the core Related to pcln-design-system project (core components) label Dec 28, 2020
@AyushRathi07
Copy link
Author

Sure sir, thanks for letting me know.

@craigpalermo craigpalermo linked an issue Jan 8, 2021 that may be closed by this pull request
@BeniCheni
Copy link
Collaborator

BeniCheni commented Jan 8, 2021

👋🏼 @AyushRathi07, thanks for contributing. A few thoughts have been shared w. other authors that we'd just give you a collective feedback, to hopefully, guide the PR to work towards completion. PTAL?

  • we definitely could correlate to the use case for a responsive image; although, we've already have Image written in our common adaptation of styled-component and styled-system. Wonder if you could coalesce your feature into the code of Image
  • ResponsiveImage, as a name, may change in future round of reviews with feedback from other authors, but it'd be possible to start with making an alias of ResponsiveImage

We wonder if you could take a look at Image, and try to edit for the next round of review?
cc/ @craigpalermo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to pcln-design-system project (core components)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image component should support a collection of source URLs
3 participants