carbon-design-system / carbon-react-native

The React Native implementation of the Carbon Design System
https://carbon-design-system.github.io/carbon-react-native/
Apache License 2.0
30 stars 8 forks source link

[Design review] Navigation list #44

Closed 8OYx4JT4MsadkJ5CPwN52Fdm7fbRLSNYd0h4y5I closed 2 years ago

8OYx4JT4MsadkJ5CPwN52Fdm7fbRLSNYd0h4y5I commented 2 years ago

Thanks a lot for adding this component.

Specs Here is some visual specification. https://www.figma.com/file/YMg6MNB1JFF73IFZkrx5P1/(v11-ALPHA)-Light-Theme---Carbon-Native-Mobile?node-id=2017%3A8059

image

Review Here are some checks I've made, basically only the red ones are something to look at.

WYXEhlEtzb2Gfe6HgrQLAf6MeMEatE77yYzHHIH commented 2 years ago

For right icon that is green. Isn’t the full component what is pressable. You can’t have pressable in pressable. Do you want that icon to be a button instead (we have to remove the onClick on the parent so they can’t be both pressable).

8OYx4JT4MsadkJ5CPwN52Fdm7fbRLSNYd0h4y5I commented 2 years ago

Hi, If you check the link to the iOS calendar above, you can see some screenshots there directly from Apple's app - Left side of the cell is basically a checkbox (you can select or deselect the item) and the Info icon on the right leads to a different screen (some details).

If the pressable can be embedded in another one, than the only solution I see is splitting areas, but because chevron does the same as the left side of the cell then this is not elegant at all. Let's leave it for now with no change and see if the community comes up with a need for it. I see some apps having a bunch of icon buttons with separate actions, but they don't use RN and their cell is not interactive, just the buttons.

WYXEhlEtzb2Gfe6HgrQLAf6MeMEatE77yYzHHIH commented 2 years ago

OK. Will hold off on nested pressables until there is need from a team. For DataTable we change the parent wrap for a row if you want inner pressables which works as expected. But it is a messy use case. When someone needs this we will revisit when they open the request. :)

WYXEhlEtzb2Gfe6HgrQLAf6MeMEatE77yYzHHIH commented 2 years ago

Are the header/footer separate components now? The header could be re-used for example in the left drawer/panel component etc.

Everything is broken into small components pieces. So hopefully no limiting for using as children or in custom use cases.

WYXEhlEtzb2Gfe6HgrQLAf6MeMEatE77yYzHHIH commented 2 years ago

NavigationList component introduced as wrapper and handles lastItem style overriding.

Check GIF for changes. As request for features come in we can constantly expand this with the additional features. But we will test the water for needs before muddying the component.

android2