Closed YohannParis closed 3 years ago
The design should not be identical to the models' selection. The border highlight is important, but in that view, the checkbox input is more confusing than anything. A better solution would be to add a highlight prop to the Card component that uses the same border design as the checked prop.
This is a really good point @YohannParis . In the Models view, we have checkboxes to inform the user that she can do multiple selection of cards (you can select more than one model and compare them. This feature is a WIP but that's the goal). In the Knowledge view, we only support single selection of cards so the checkboxes in this view, as you say, don't add much. We probably want to have two props: checked
and highlighted
. Highlighted applies to both views but only checked applies to the Models view because of the multi-selection. Any thoughts? @mj3cheun ?
Yup having separate highlight and checked props works for me
Great. Should I update this PR, or make another one? How do guys prefer to do it?
Great. Should I update this PR, or make another one? How do guys prefer to do it?
Since you're in the area, what about addressing it in this PR? Up to you.
I updated the code and the PR copy to reflect our conversation. This is ready for review.
Thanks!
What?
fix #149
Why?
To match the
Knowledge
view card selection behaviour of theModels
view.How?
card
to include achecked
andhighlighted
properties.Home
view to theCard
component itself.cards
anddrilldownData
of theDocsCards
component to beCardInterface
.highlighted
property on the cards if the drilldown panel is open.Testing?
Screenshot
Anything Else?
~The design should not be identical to the models' selection. The border highlight is important, but in that view, the checkbox input is more confusing than anything. A better solution would be to add a
highlight
prop to theCard
component that uses the same border design as thechecked
prop.~ This has been updated by creating a newhighlighted
property to the card component.