dooboolab-community/dooboo-ui

Enhance [Accordion] usability

luke-hanwook opened this issue · 8 comments

luke-hanwook commented

I'm uncertain about how to use Accordion component.

if it's just expandable component, simple is better good. (one summary, one detail) like a below.
https://material-ui.com/components/accordion/#simple-accordion
image

if it's like a list, that component provides a lot of feature. (not only display but also clickable, linking..)
https://reactnativeelements.com/docs/next/main/usage/ListItem/ListItem/#listitem-accordion
https://material-ui.com/components/lists/#nested-list

To inject data props is very simple and awesome. (Unlike other UI framework, it's dooboo-ui's differentiation)
but i think this feature is limited in at least accordion. (because datum bodies type is string[])

In order to get a direction of how to improve, i would like to discuss them further.

luke-hanwook commented

Sorry I think it can resolve to use render prefix props.
this component can use very variously! 👍🏻
See the No.4 section at Coding guidelines

  • 👍1
luke-hanwook commented

When I use collapseOnStart and toggleElement props on [Accordion], I was a little confused.
So I think it's good to modify consistent props name.
ex) collapseOnStart -> defaultExpanded, toggleElement -> expandButtonElement.
But I'm concerned because of existing user with dooboo-ui.

hyochan commented

@DevYuns Spotting initial Accrodion writer for this!

  • 👍1
luke-hanwook commented

image

May I add a border on this component?
It can distinguish the component from background color.

yujonglee commented

@luke-hanwook I think it can be done with styles.titleContainer and styles.bodyContainer.
However, it seems like translateY does not reflect the given border, so it looks strange.

So I think you can try fixing this. It will be a great help.

AB69B38D-1BD8-41DA-9640-8086BB2D1F63_4_5005_c

  • 👍1
luke-hanwook commented

I think using transform property to animated value is problem. (this issue too)
It's also problem to exist unnecessary areas because of transition area. (like below image)
So i've replaced animated value with height property.
height value of hidden list component is 0 when component is collapse and the other thing is component height when mounted.
but height property is unsupported in animated style of RN.

so i'm searching for the solution.
currently

  1. use the react-native-reanimated
  2. set up the usenativeDriver property value to false (need to test of performance. ref link)

Please someone tell me any suggestion or insight! it's big help to me.

image

yujonglee commented

@DevYuns Can you share your thought on this?

luke-hanwook commented

@yujong-lee
I had a talk about sync for animation action of this component with @hyochan. (Our choice is no.2)
and I had a git rebase your code to use theme in styled component.

  • 👍1