BA-2391: create native tabs component #241
Conversation
Ronan-Fernandes
requested review from
anicioalexandre,
deboracosilveira and
priscilladeroode
as code owners
|
WalkthroughThis update introduces new Tabs and Tab components into the design system along with their supporting files. The changes include the creation of functional components for individual tabs and a container for multiple tabs, the addition of corresponding styles and types, the implementation of a shared context for managing tab states, and updates to the package's changelog and version. The components use hooks to integrate with the theme and context, enabling dynamic styling based on the selected state. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant TC as Tab Component
participant CNTX as TabsContext
participant TPC as Tabs Component
U->>TC: Clicks on Tab button
TC->>CNTX: Reads current tab value and onChange handler
TC->>TPC: Invokes onChange callback with new tab value
TPC-->>CNTX: Updates currentValue in context
CNTX-->>TC: Renders Tab with updated selection state
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/design-system/components/native/tabs/Tab/styles.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/design-system/.eslintrc.js
packages/design-system/components/native/tabs/Tab/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/design-system/.eslintrc.js
packages/design-system/components/native/tabs/Tabs/styles.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/design-system/.eslintrc.js
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/design-system/components/native/tabs/Tab/index.tsx (1)
23-29: Missing accessibility properties for the tab component.The tab component should have appropriate ARIA roles and properties to ensure proper accessibility.
return ( <View style={styles.baseTabContainer}> - <Button mode="text" onPress={() => onChange(value)}> + <Button + mode="text" + onPress={() => onChange(value)} + accessibilityRole="tab" + accessibilityState={{ selected: isSelected }} + > <Text variant="subtitle2" color={isSelected ? selectedColor : unselectedColor}> {label} </Text> </Button> </View> )packages/design-system/components/native/tabs/Tabs/index.tsx (3)
9-12: Consider handling undefinedonChangewith a default value.The
onChangeprop doesn't have a default value, which could lead to errors if not provided. Since you're passing it to child components, it should have a fallback.-const Tabs: FC<TabsProps> = ({ onChange, value, children, style, ...props }) => { +const Tabs: FC<TabsProps> = ({ onChange = () => {}, value, children, style, ...props }) => { const theme = useTheme() const styles = createStyles(theme)
13-22: Add type safety for child elements.The component assumes that any valid React element can receive the
onChangeandcurrentValueprops. Consider adding a more specific type check to ensure only Tab components are processed.const renderChildren = () => React.Children.map(children, (child) => { if (React.isValidElement(child)) { + // Check if child is a Tab component or has the necessary props shape + if (typeof child.type === 'function' && 'value' in child.props) { return React.cloneElement(child as React.ReactElement<TabProps>, { onChange, currentValue: value, }) + } + console.warn('Tabs component only accepts Tab components as children') + return child; } return null })
24-27: Add accessibility roles for the tabs container.The tabs container should have appropriate ARIA roles to ensure proper accessibility.
return ( - <View style={[styles.baseTabsContainer, style]} {...props}> + <View + style={[styles.baseTabsContainer, style]} + accessibilityRole="tablist" + {...props} + > {renderChildren()} </View> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/components/__generated__/ActivityLogsFragment.graphql.tsis excluded by!**/__generated__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/design-system/components/native/tabs/Tab/index.tsx(1 hunks)packages/design-system/components/native/tabs/Tab/styles.ts(1 hunks)packages/design-system/components/native/tabs/Tab/types.ts(1 hunks)packages/design-system/components/native/tabs/Tabs/index.tsx(1 hunks)packages/design-system/components/native/tabs/Tabs/styles.ts(1 hunks)packages/design-system/components/native/tabs/Tabs/types.ts(1 hunks)packages/design-system/components/native/tabs/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/design-system/components/native/tabs/Tabs/types.ts (1)
packages/design-system/components/native/tabs/Tab/types.ts (1) (1)
TabProps(3-10)
packages/design-system/components/native/tabs/Tabs/styles.ts (1)
packages/design-system/components/native/tabs/Tab/styles.ts (1) (1)
createStyles(5-11)
🔇 Additional comments (5)
packages/design-system/components/native/tabs/Tabs/types.ts (1)
1-8: Interface design looks clean and appropriateThe
TabsPropsinterface properly extendsViewPropsand includes the essential properties for a tabs container component. The type definition correctly specifies thatchildrenshould be React elements that conform to theTabPropsinterface.A couple of observations:
- The
onChangefunction is required, which is appropriate for controlled components- The typed array for children will ensure type safety when rendering Tab components
packages/design-system/components/native/tabs/Tabs/styles.ts (1)
5-16: Style implementation follows React Native best practicesThe
createStylesfunction is well-implemented, creating a container with appropriate flex layout properties and a subtle bottom border to provide visual structure for the tabs.The use of theme values for colors maintains consistency with the design system, and the explicit gap value helps ensure proper spacing between tab elements.
packages/design-system/components/native/tabs/Tab/types.ts (1)
3-10: Well-structured interface with appropriate optionalityThe
TabPropsinterface is well-designed with:
- Required properties (
value,label) for core functionality- Optional callbacks and styling properties for flexibility
- Type-safe color selection using
keyof ObjectTokenThis approach allows for both controlled (when parent manages state via
onChangeandcurrentValue) and uncontrolled usage of the Tab component.packages/design-system/components/native/tabs/Tab/styles.ts (1)
5-11: Clean conditional styling based on selection stateThe style implementation effectively uses the
isSelectedparameter to conditionally apply bottom border styling, creating a clear visual indicator for the active tab.This approach is performant and follows React Native best practices by:
- Using conditional expressions for dynamic styles
- Leveraging theme colors for consistency
- Keeping the style object simple and focused
packages/design-system/components/native/tabs/index.ts (1)
1-4: Exports look good and follow standard patterns.The barrel file properly exports both the Tabs and Tab components as named exports, along with their type definitions. This approach allows users to import everything needed from a single entry point.
anicioalexandre
added
needs changes
Ronan-Fernandes
force-pushed
the
BA-2391-rn-create-tabs-component
branch
2 times, most recently
from
d24db23 to
57588d6
Compare
anicioalexandre
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/design-system/components/native/tabs/Tab/styles.ts (1)
5-11: Consider adding more styling properties to the tab component.The style currently only defines a bottom border for selected tabs. Consider adding additional styling properties like padding, alignment, width/height, and possibly text color changes based on selection state for a more complete and visually consistent tab component.
export const createStyles = (theme: Theme, isSelected: boolean) => StyleSheet.create({ baseTabContainer: { borderBottomColor: theme.colors.object.low, borderBottomWidth: isSelected ? 2 : 0, + paddingVertical: 8, + paddingHorizontal: 16, + alignItems: 'center', + justifyContent: 'center', }, })packages/design-system/components/native/tabs/Tabs/context.tsx (1)
7-10: Add error handling to the context hook.The TabsContext is implemented correctly, but consider adding error handling to the useTabsContext hook to ensure it's used within a TabsContext.Provider.
export const TabsContext = createContext<TabsContextValue>({ currentValue: '', onChange: () => {}, }) -export const useTabsContext = () => useContext(TabsContext) +export const useTabsContext = () => { + const context = useContext(TabsContext) + if (context === undefined) { + throw new Error('useTabsContext must be used within a TabsProvider') + } + return context +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/components/__generated__/ActivityLogsFragment.graphql.tsis excluded by!**/__generated__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
packages/components/CHANGELOG.md(1 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/native/tabs/Tab/index.tsx(1 hunks)packages/design-system/components/native/tabs/Tab/styles.ts(1 hunks)packages/design-system/components/native/tabs/Tab/types.ts(1 hunks)packages/design-system/components/native/tabs/Tabs/context.tsx(1 hunks)packages/design-system/components/native/tabs/Tabs/index.tsx(1 hunks)packages/design-system/components/native/tabs/Tabs/styles.ts(1 hunks)packages/design-system/components/native/tabs/Tabs/types.ts(1 hunks)packages/design-system/components/native/tabs/index.ts(1 hunks)packages/design-system/package.json(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/components/CHANGELOG.md
- packages/wagtail/package.json
- packages/components/package.json
- packages/design-system/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/design-system/components/native/tabs/Tab/types.ts
- packages/design-system/components/native/tabs/Tabs/styles.ts
- packages/design-system/components/native/tabs/index.ts
🧰 Additional context used
🧬 Code Definitions (3)
packages/design-system/components/native/tabs/Tabs/context.tsx (1)
packages/design-system/components/native/tabs/Tabs/types.ts (1)
TabsContextValue(12-15)
packages/design-system/components/native/tabs/Tabs/types.ts (1)
packages/design-system/components/native/tabs/Tab/types.ts (1)
TabProps(3-8)
packages/design-system/components/native/tabs/Tab/index.tsx (3)
packages/design-system/components/native/tabs/Tab/types.ts (1)
TabProps(3-8)packages/design-system/components/native/tabs/Tabs/context.tsx (1)
useTabsContext(12-12)packages/design-system/components/native/tabs/Tab/styles.ts (1)
createStyles(5-11)
🔇 Additional comments (6)
packages/design-system/CHANGELOG.md (1)
3-8: Documentation of new Tabs component looks good.The changelog entry properly documents the addition of custom Tabs Components to the design system as a patch change in version 1.0.10.
packages/wagtail/CHANGELOG.md (1)
3-8: Dependency update is properly documented.The changelog correctly documents updating the design-system dependency to version 1.0.10, which contains the new tabs components.
packages/design-system/components/native/tabs/Tabs/types.ts (1)
1-15: Well-structured type definitions with proper typing.The types are well-defined and follow best practices. The
TabsPropsinterface appropriately extendsViewPropsand properly types the tab components' children asReactElement<TabProps>[]. TheTabsContextValuetype clearly defines the shape of the context value needed for tabs state management.packages/design-system/components/native/tabs/Tabs/index.tsx (1)
9-21: Good implementation using context for state sharing.The implementation follows best practices by:
- Using context for state sharing (as advised in previous comments)
- Memoizing the context value to optimize performance
- Properly handling styles by merging base styles with custom styles
- Following a clean component structure
This approach is preferable to using
cloneElementwhich can impact performance with multiple children.packages/design-system/components/native/tabs/Tab/index.tsx (2)
11-26: Component name is now correct and implementation is solid.The component has been properly renamed from "Tabs" to "Tab" as noted in previous comments. The implementation follows best practices by:
- Using the context hook to access shared state
- Determining selection state by comparing values
- Applying appropriate styles based on selection state
- Providing sensible defaults for color props
The component correctly renders a button that triggers the tab change and visually indicates the current selection state.
28-28: Default export now matches component name.The export has been properly updated to match the "Tab" component name, resolving the issue mentioned in previous reviews.
Ronan-Fernandes
force-pushed
the
BA-2391-rn-create-tabs-component
branch
from
d0f4d37 to
bf3d044
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/design-system/components/native/tabs/Tab/index.tsx (1)
11-26: Tab component implementation looks good with a few suggestions for improvement.The implementation correctly renders a tab with different styling based on selection state. I have a few suggestions for improvement:
- Consider adding accessibility props to improve screen reader support:
<View style={styles.baseTabContainer}> - <Button mode="text" onPress={() => onChange(value)}> + <Button + mode="text" + onPress={() => onChange(value)} + accessibilityRole="tab" + accessibilityState={{ selected: isSelected }} + > <Text variant="subtitle2" color={isSelected ? selectedColor : unselectedColor}> {label} </Text> </Button> </View>
- You might want to add a visual indicator for the currently selected tab that doesn't rely solely on color, to improve accessibility for users with color vision deficiencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/components/__generated__/ActivityLogsFragment.graphql.tsis excluded by!**/__generated__/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/native/tabs/Tab/index.tsx(1 hunks)packages/design-system/components/native/tabs/Tab/styles.ts(1 hunks)packages/design-system/components/native/tabs/Tab/types.ts(1 hunks)packages/design-system/components/native/tabs/Tabs/context.tsx(1 hunks)packages/design-system/components/native/tabs/Tabs/index.tsx(1 hunks)packages/design-system/components/native/tabs/Tabs/styles.ts(1 hunks)packages/design-system/components/native/tabs/Tabs/types.ts(1 hunks)packages/design-system/components/native/tabs/index.ts(1 hunks)packages/design-system/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/design-system/CHANGELOG.md
- packages/design-system/package.json
- packages/design-system/components/native/tabs/Tabs/styles.ts
- packages/design-system/components/native/tabs/Tab/types.ts
- packages/design-system/components/native/tabs/Tabs/context.tsx
- packages/design-system/components/native/tabs/Tab/styles.ts
- packages/design-system/components/native/tabs/index.ts
- packages/design-system/components/native/tabs/Tabs/index.tsx
- packages/design-system/components/native/tabs/Tabs/types.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/design-system/components/native/tabs/Tab/index.tsx (3)
packages/design-system/components/native/tabs/Tab/types.ts (1)
TabProps(3-8)packages/design-system/components/native/tabs/Tabs/context.tsx (1)
useTabsContext(12-12)packages/design-system/components/native/tabs/Tab/styles.ts (1)
createStyles(5-11)
🔇 Additional comments (2)
packages/design-system/components/native/tabs/Tab/index.tsx (2)
1-9: Component imports are well-organized.Good practice to separate React imports, design system components, and local imports into different groups.
28-28: Export naming has been corrected.The component name and export now correctly match as
Tab, which is appropriate for a component representing a single tab.
__package_name__package update -v __package_version__Summary by CodeRabbit