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

[FloatingActionButton] implementing floatingActionButton component #63

Closed
Juyeong-Byeon opened this issue Aug 10, 2021 · 11 comments
Closed
Assignees
Labels
1️⃣ good first issue Good for newcomers 🥺 feature request Request a new feature

Comments

@Juyeong-Byeon
Copy link
Contributor

Juyeong-Byeon commented Aug 10, 2021

Floating action button(FAB) is a very common component in touch based devices.

material design

There are many libraries that supports floating action button, and it is good for highlights the most important action on the page.

image

I think we should support FAB.

I'm proposing FAB that has the following features:

  1. that can be placed in an absolute position on the screen.
  2. that allows you to set icons with content
  3. that can show additional action buttons when selected
@Juyeong-Byeon
Copy link
Contributor Author

I think it's similar to IconButton component.
But it has difference in feature 1, 3
If you decide to implement it, assign it to me.

@hyochan hyochan added 1️⃣ good first issue Good for newcomers 🥺 feature request Request a new feature labels Aug 10, 2021
@hyochan
Copy link
Member

hyochan commented Aug 10, 2021

I think it's similar to IconButton component.
But it has difference in feature 1, 3
If you decide to implement it, assign it to me.

Assigned 🚀

@yujonglee
Copy link
Contributor

yujonglee commented Aug 10, 2021

@wndudqus
Looks good to me.
Like you mentioned, I think It will be nice to get component like Button and IconButton as props.

<Floating {...props}>
  <Button onClick={...}>
</Floating>

or for the feature 3,

<Floating
  {...props}
  main={<Button onClick-{...}/>}
  sub={[<IconButton name='abc'/>, <IconButon name='bcd'/>]}
>

How do you think?

I'm looking forward to your work. 😄

@Juyeong-Byeon
Copy link
Contributor Author

@yujong-lee
Thank you. I was thinking in the same way

@hyochan
Copy link
Member

hyochan commented Aug 13, 2021

@wndudqus
Looks good to me.
Like you mentioned, I think It will be nice to get component like Button and IconButton as props.

<Floating {...props}>
  <Button onClick={...}>
</Floating>

or for the feature 3,

<Floating
  {...props}
  main={<Button onClick-{...}/>}
  sub={[<IconButton name='abc'/>, <IconButon name='bcd'/>]}
>

How do you think?

I'm looking forward to your work. 😄

@yujong-lee I just want to point out DRY for same elements in sub. Same IconButton should not be repeated.

@hyochan
Copy link
Member

hyochan commented Aug 13, 2021

If someone wants to support custom element in specifix index, it can be provided like below.

<FAB
   renderItems={(item, i) => {
    if (i === 3) return <MyView />;
    return <View />;
  }
...
/>

Just my idea 😉

@Juyeong-Byeon
Copy link
Contributor Author

Juyeong-Byeon commented Aug 13, 2021

I think you meant this.

<FAB
   renderItems={(item, i) => {
    if (i === 3) return <MyView />;
    return item;//return original Component
  }
...
/>

Did I get it right?
If I get It right,I think that is wonderful Idea. Thanks!

@yujonglee
Copy link
Contributor

yujonglee commented Aug 13, 2021

@wndudqus
Looks good to me.
Like you mentioned, I think It will be nice to get component like Button and IconButton as props.

<Floating {...props}>
  <Button onClick={...}>
</Floating>

or for the feature 3,

<Floating
  {...props}
  main={<Button onClick-{...}/>}
  sub={[<IconButton name='abc'/>, <IconButon name='bcd'/>]}
>

How do you think?
I'm looking forward to your work. 😄

@yujong-lee I just want to point out DRY for same elements in sub. Same IconButton should not be repeated.

@hyochan

Thanks for sharing your thought!

Yes. repeating is bad practice. But I think repeating doesn't matter here because I can always remove it using function returning array of components. (for simple case map will do). But It will end up writing render function, so eventually your approach might be better.

So now, I think the key is how renderFuction should be defined.

@hyochan
Copy link
Member

hyochan commented Aug 13, 2021

I think you meant this.

<FAB
   renderItems={(item, i) => {
    if (i === 3) return <MyView />;
    return item;//return original Component
  }
...
/>

Did I get it right?
If I get It right,I think that is wonderful Idea. Thanks!

The item is a data not an Element. You might want to refer to FlatList. This is a great component made by Facebook!

Also, not only renderItem but providing default style without it would be even better. Then renderItem can be optional.

@Juyeong-Byeon Juyeong-Byeon mentioned this issue Aug 14, 2021
4 tasks
@Juyeong-Byeon
Copy link
Contributor Author

Juyeong-Byeon commented Aug 17, 2021

I created PR about this issue.
I want to talk about structure of this FAB component.
Will it be good to proceed with this structure?

If you have some time please check it out.
Or should I implement more specifically?

@yujonglee
Copy link
Contributor

Resolved with #67.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1️⃣ good first issue Good for newcomers 🥺 feature request Request a new feature
Projects
None yet
Development

No branches or pull requests

3 participants