telus / tds-community

TELUS Design System Community Components
https://tds.telus.com/community/index.html
MIT License
26 stars 7 forks source link

new: Testimonials Card #90

Closed joperron closed 5 years ago

joperron commented 6 years ago

Problem statement

We need the testimonials card for the Telus Health website we are building with Site Builder. It exists in the Sketch components, but not in the TDS React components apparently.

Recommendation

Designs

Attached are screens from DSM and in the context of our project:

screen shot 2018-08-17 at 1 41 24 pm screen shot 2018-08-17 at 1 19 02 pm

Meta

wascloud commented 6 years ago

DPA Group: Is the request for the entire card? or just the image in a circle?

joperron commented 6 years ago

Up to you, would be easier to just do the entire card instead of fragmenting what exists, your call

varunj90 commented 6 years ago

@joperron , @DougTelus will be happy to help you w.r.t the design

agorovyi commented 6 years ago

While analyzing this request I noticed that TDS Core-Components have everything that is needed to put this card together. The piece I thought that was missing is the circle image but I found that you can pass rounded="circle" prop to <Image> Component: https://tds.telus.com/components/index.html#image

The only challenge that is left is how to make this image to be Lazy Loaded.

My question is do we still want to put this card together as a standalone <Testimonial> Component?

agorovyi commented 6 years ago

@githubjosh @xingbo828 We need your input here. We were discussing TDS <Image> Component and that it's missing LazyLoad functionality and were trying to come up with a solution on how to go around this.

There are two proposals that came up in the discussion:

  1. Merge <ResponsiveImage> Component from redux-contentful with TDS <Image> Component and have one unified (potentially) TDS Component.
  2. Copy over LazyLoad functionality from <ResponsiveImage> Component into TDS <Image>. However, this approach will duplicate the need for long-term maintenance of LazyLoad functionality in both places.

Could you please provide your thoughts and recommendations on this?

githubjosh commented 6 years ago

@agorovyi

withLazyLoad.jsx in redux-contentful is just a wrapper (HOC). You could potentially import this into TDS to wrap your images.

Notes:

Alternatively, you could simply import the react-lazy-load module and wrap your TDS image component.

`import LazyLoad from 'react-lazy-load'; ...

...

`

From my POV, the tangible benefits of using the existing HOC are:

I'd start with the HOC, but I'm mostly indifferent. The LOE either way is nominal

agorovyi commented 6 years ago

@githubjosh

Thanks for the input. I like the idea of using withLazyLoad.jsx HOC from redux-contentful. In this case we can keep the LazyLoad maintenance in one place. However, I've just realized that redux-contentful is a Private repo. I'm wondering how is it going to work with TDS considering it's a Public repo?

@theetrain do you think it's okay to have dependencies from Private repos in TDS? If not I guess the only solution to this would be to pull react-lazy-load separately into TDS-core.

githubjosh commented 6 years ago

@agorovyi Alternatively, you could pull withLazyLoad.jsx out of redux-contentful, move it into TDS, then make this TDS component a dependency of redux-contentful for use with responsiveImage: i.e. flip the paradigm

agorovyi commented 6 years ago

@githubjosh This is it. Like this idea.

varunj90 commented 6 years ago

@agorovyi will be story writing this!

agorovyi commented 6 years ago

User Story

As a TDS-Community consumer, I want to display a Testimonial Component that will consist of Card, Image, and Paragraph Core Components put together in one standalone Component as per the approved design.

Acceptance Criteria: Component

Tech Notes

theetrain commented 6 years ago

Do we need to have an option without the image or image must be a required element?

Since images are optional, we recommend providing a prop interface that accepts the core-image component.

What elements must be required in this Component? My suggestion: testimonial text, testimonial author title

Looking at an example, I see (1) paragraph text for the testimonial, (2) bold text for the author, and (3) subtext for the author title are always required.

joperron commented 6 years ago

Hey guys,

Just wanted to check in and ask how close we are to starting dev on this, and who is going to do it?

agorovyi commented 6 years ago

@joperron I've already submitted a PR (#114) for it. It will most likely have some revisions based on provided feedback as it is my first one here.

theetrain commented 6 years ago

Update, we're going to need another pull request to allow the <Image/> component to be inserted by the consuming developer, while creating maximum boundaries in the testimonial card so that images do not exceed 60x60px.

theetrain commented 6 years ago

This component was released: @tds/community-testimonial@1.0.0

Christina-Lo commented 6 years ago

This component has been released in Community DSM as well https://telus.invisionapp.com/dsm/telus/tds-community/folder/components/5b0eee0dbd61de0011520059

varunj90 commented 5 years ago

assignees have been added on closed issue to reflect power level (points) on community heroes board (coming soon!)