BA-2279: mobile settings menu #223
Conversation
anicioalexandre
requested review from
deboracosilveira and
priscilladeroode
as code owners
|
WalkthroughThis PR introduces extensive updates across multiple packages. It adds new avatar and icon components with associated types and styles, and refactors the AppBar component by replacing deprecated props with new ones for handling back and close actions. The ProfileSettings component is restructured with a simplified layout and stylesheet modifications. Enhancements are made to navigation layouts with new properties and dynamic behaviors. Additionally, package version numbers and dependency versions are updated across the design system, components, and workspace, with corresponding changelog entries revised. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppBar
participant NavigationHandler
User->>AppBar: Tap Back or Close button
AppBar->>NavigationHandler: Invoke onBack/onClose callback
NavigationHandler-->>User: Perform navigation update
sequenceDiagram
participant User
participant BottomNavigationLayout
participant Tabs
User->>BottomNavigationLayout: Navigate to route
BottomNavigationLayout->>BottomNavigationLayout: Check if route ∈ routesWithoutHeader
alt Route without header
BottomNavigationLayout->>Tabs: Disable header and swipe
else Route with header
BottomNavigationLayout->>Tabs: Enable header and swipe
end
Possibly related PRs
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/components/baseapp-frontend-imports.d.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/components/modules/profiles/native/ProfileSettingsComponent/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/components/.eslintrc.js
packages/design-system/components/native/appbars/AppBar/styles.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /packages/design-system/.eslintrc.js
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
anicioalexandre
force-pushed
the
BA-2279-mobile-settings-menu
branch
from
477ae0a to
4be5947
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🧹 Nitpick comments (19)
packages/design-system/components/native/avatars/ClickableAvatar/styles.ts (1)
6-29: Consider making border radius and padding proportional to size.The styles are well-structured, but there are opportunities for improvement:
- The border radius of 50 might be too high for small avatars. Consider using
width/2for a perfect circle.- The hardcoded padding of 4 might look disproportionate for different sizes. Consider making it a percentage of the width/height.
export const createStyles = ({ colors }: Theme, { width, height }: SizeStylesOptions) => StyleSheet.create({ container: { alignItems: 'center', borderColor: colors.surface.active, - borderRadius: 50, + borderRadius: width / 2, borderWidth: 2, - height: height + 4, + height: height * 1.1, justifyContent: 'center', overflow: 'hidden', - width: width + 4, + width: width * 1.1, }, image: { - borderRadius: 50, + borderRadius: width / 2, height, width, }, imageFallbackState: { backgroundColor: colors.surface.active, - borderRadius: 50, + borderRadius: width / 2, height, width, }, })packages/design-system/components/native/avatars/ClickableAvatar/index.tsx (2)
10-16: Add loading state and accessibility props.The component props are well-defined with good defaults, but consider adding:
- A loading state for the Image component
- Accessibility props for better user experience
const ClickableAvatar: FC<ClickableAvatarProps> = ({ imageSource, onPress, height = 36, width = 36, fallbackMode = false, + accessibilityLabel = 'User avatar', + accessibilityRole = 'button', }) => {
28-34: Add loading state for Image component.Consider handling loading state for the Image component to improve user experience.
return ( - <Pressable onPress={onPress}> + <Pressable + onPress={onPress} + disabled={!onPress} + accessibilityLabel={accessibilityLabel} + accessibilityRole={accessibilityRole} + > <View style={styles.container}> - <Image source={imageSource} style={styles.image} /> + <Image + source={imageSource} + style={styles.image} + onLoadStart={() => setIsLoading(true)} + onLoadEnd={() => setIsLoading(false)} + /> + {isLoading && ( + <ActivityIndicator + style={StyleSheet.absoluteFill} + color={theme.colors.surface.active} + /> + )} </View> </Pressable> )packages/design-system/layouts/native/BottomNavigationLayout/types.ts (1)
16-16: LGTM! Consider adding JSDoc comments.The new
routesWithoutHeaderprop aligns well with the PR objectives, allowing control over header visibility for specific routes. Consider adding JSDoc comments to document the prop's purpose and expected values.+ /** Array of routes where the header should be hidden */ routesWithoutHeader?: Href[]packages/design-system/layouts/native/BottomNavigationLayout/index.tsx (2)
12-18: Consider memoizing the pathname check.The
isPathnameInTabscalculation could be memoized to prevent unnecessary recalculations on re-renders.+ const isPathnameInTabs = useMemo( + () => tabs.some(({ href }) => href === pathname), + [tabs, pathname] + ) - const isPathnameInTabs = tabs.some(({ href }) => href === pathname)
27-27: Consider documenting the backBehavior choice.The
backBehavior: 'history'setting is a significant navigation choice. Consider adding a comment explaining why this behavior was chosen over other options like 'initialRoute' or 'order'.+ {/* Using 'history' to maintain navigation stack when switching tabs */} <Tabs options={{ backBehavior: 'history' }}>packages/design-system/components/native/views/PageViewWithHeader/index.tsx (1)
8-8: MakedismissKeyboardprop configurable.The component currently forces keyboard dismissal behavior for all instances. Consider making this configurable by accepting it as a prop with a default value, allowing consumers to override when needed:
-const PageViewWithHeader: FC<PageViewWithHeaderProps> = ({ style, ...props }) => ( - <View style={[styles.container, style]} {...props} dismissKeyboard /> +const PageViewWithHeader: FC<PageViewWithHeaderProps> = ({ style, dismissKeyboard = true, ...props }) => ( + <View style={[styles.container, style]} {...props} dismissKeyboard={dismissKeyboard} />Don't forget to update the types file to include the new prop:
interface PageViewWithHeaderProps extends ViewProps { dismissKeyboard?: boolean; }packages/design-system/components/native/appbars/AppBar/index.tsx (2)
22-29: Consider improving accessibility by adding labels and test IDs for the back button.
This enhances test automation and screen reader accessibility.Here’s a sample diff for line 25 to line 27:
- <IconButton onPress={onBack}> + <IconButton onPress={onBack} accessibilityLabel="Go back" testID="appbar-back-button"> <BackIcon /> </IconButton>
34-40: Add labels/test IDs for the close button to improve accessibility and testing.
Similar to the back button, providing a label supports screen readers while test IDs support automated UI tests.Consider the following diff for line 35 to line 37:
- <IconButton onPress={onClose}> + <IconButton onPress={onClose} accessibilityLabel="Close" testID="appbar-close-button"> <CloseIcon /> </IconButton>packages/design-system/layouts/native/AppbarNavigationLayout/index.tsx (1)
10-15: Consider adding accessibility support.To improve accessibility, consider adding:
accessibilityRole="navigation"to the container ViewaccessibilityLabelfor the AppBar titleconst AppbarNavigationLayout: FC<AppbarNavigationLayoutProps> = ({ title, onBack, onClose }) => ( - <View style={styles.container} dismissKeyboard> + <View + style={styles.container} + dismissKeyboard + accessibilityRole="navigation" + > - <AppBar title={title} onBack={onBack} onClose={onClose} /> + <AppBar + title={title} + onBack={onBack} + onClose={onClose} + accessibilityLabel={`${title} navigation header`} + /> <Slot /> </View> )packages/design-system/components/native/icons/ProfileSettingsIcon/index.tsx (1)
25-26: Consider memoizing width/height calculations.The width and height calculations in the Rect component are performed on every render. Consider memoizing these values using
useMemoto optimize performance.+import { FC, useMemo } from 'react' ... const ProfileSettingsIcon: FC<SvgIconProps> = ({ ... }) => { const { colors } = useTheme() + const rectDimensions = useMemo(() => ({ + width: Number(width) / 2.5, + height: Number(height) / 2.5 + }), [width, height]) ... <Rect x="4.66675" y="1.11523" - width={Number(width) / 2.5} - height={Number(height) / 2.5} + width={rectDimensions.width} + height={rectDimensions.height} rx="3.33333"packages/design-system/components/native/icons/ChevronIcon/index.tsx (2)
19-20: Ensure consistent color usage across icons.The default color uses
colors.object.lowwhile other icons usecolors.object.high. Consider usingcolors.object.highfor consistency.- const defaultColor = color ?? colors.object.low + const defaultColor = color ?? colors.object.high
22-27: Consider extracting rotation constants.The rotation degrees are hardcoded in the component. Consider extracting these values to a constants file for better maintainability and reusability.
+// In a new file: packages/design-system/components/native/icons/ChevronIcon/constants.ts +export const ROTATE_DEGREES: RotateRecord = { + right: '180deg', + left: '0deg', + down: '270deg', + up: '90deg', +} as const +// In ChevronIcon/index.tsx +import { ROTATE_DEGREES } from './constants' ... - const rotateDeg: RotateRecord = { - right: '180deg', - left: '0deg', - down: '270deg', - up: '90deg', - } + const rotateDeg = ROTATE_DEGREESpackages/design-system/components/native/icons/BellIcon/index.tsx (1)
8-38: Add accessibility attributes for screen readers.Consider enhancing accessibility by adding ARIA attributes and role.
const BellIcon: FC<SvgIconProps> = ({ isActive = false, color, width = '20', height = '21', + 'aria-label': ariaLabel = 'Notifications', + role = 'img', ...props }) => { const { colors } = useTheme() const defaultColor = color ?? colors.object.high const svgColor = isActive ? colors.primary.high : defaultColor return ( <Svg width={width} height={height} viewBox="0 0 20 21" color={svgColor} fill="none" + role={role} + aria-label={ariaLabel} {...props} >packages/design-system/components/native/icons/BlockIcon/index.tsx (1)
8-36: Add accessibility attributes and optimize SVG.Two suggestions for improvement:
- Add ARIA attributes for accessibility
- Remove redundant fill="none" on Svg element since paths use fill="currentColor"
const BlockIcon: FC<SvgIconProps> = ({ isActive = false, color, width = '20', height = '21', + 'aria-label': ariaLabel = 'Block', + role = 'img', ...props }) => { const { colors } = useTheme() const defaultColor = color ?? colors.object.high const svgColor = isActive ? colors.primary.high : defaultColor return ( <Svg width={width} height={height} viewBox="0 0 20 21" color={svgColor} - fill="none" + role={role} + aria-label={ariaLabel} {...props} >packages/design-system/components/native/icons/SecurityIcon/index.tsx (1)
8-47: Add accessibility attributes and fix line endings.Two suggestions for improvement:
- Add ARIA attributes for accessibility
- Ensure consistent line endings in the file
const SecurityIcon: FC<SvgIconProps> = ({ isActive = false, color, width = '20', height = '21', + 'aria-label': ariaLabel = 'Security', + role = 'img', ...props }) => { const { colors } = useTheme() const defaultColor = color ?? colors.object.high const svgColor = isActive ? colors.primary.high : defaultColor return ( <Svg width={width} height={height} viewBox="0 0 20 21" color={svgColor} fill="none" + role={role} + aria-label={ariaLabel} {...props} >packages/components/modules/profiles/native/ProfileSettingsComponent/index.tsx (3)
64-74: Consider abstracting file upload functionality.There are TODOs indicating the need to abstract file upload functionality and reuse the mutation query. This would improve code maintainability and reduce duplication.
Would you like me to help create a shared file upload utility that can be reused across the application?
298-299: TODO: Implement formatted phone number input.The comment indicates a need for a formatted phone number input. Consider using a library like
react-native-phone-number-inputfor proper phone number formatting and validation.Would you like me to help implement the phone number formatting functionality?
229-232: Implement the sheet changes handler.The TODO comment for handling sheet changes is currently only logging the index. Consider implementing proper handling for sheet state changes.
const handleSheetChanges = useCallback((index: number) => { - // TODO: handle sheet changes - console.log('index', index) + // Handle sheet state based on index + if (index === -1) { + // Sheet is closed + setFieldType('image') // Reset to default + } }, [])
🛑 Comments failed to post (2)
packages/design-system/components/native/views/View/index.tsx (1)
9-9: 💡 Verification agent
⚠️ Potential issue🧩 Analysis chain
Breaking Change: Default keyboard dismissal behavior changed.
Changing the default value of
dismissKeyboardfromtruetofalseis a breaking change that could affect existing components relying on the previous behavior.Let's verify the impact on existing components:
🏁 Script executed:
#!/bin/bash # Search for View components without explicit dismissKeyboard prop ast-grep --pattern '<View $$$>' # Search for direct imports of the View component rg -l "import.*View.*from.*views/View"Length of output: 4717
Critical Breaking Change: Default
dismissKeyboardnow set tofalseWe've verified that many
<View>component instances across the codebase lack an explicitdismissKeyboardprop. For example, files like:
packages/design-system/components/native/displays/FeedbackScreen/index.tsxpackages/design-system/components/native/displays/LoadingScreen/index.tsxpackages/design-system/components/native/inputs/TextInput/index.tsx- and others
will now receive a default value of
falserather than the previoustrue. This change could lead to unintended behavior in components that relied on the old default. Please review these instances and either update them by explicitly settingdismissKeyboardif the new behavior isn’t desired, or consider reverting the default change to maintain backward compatibility.packages/design-system/components/native/icons/CloseIcon/index.tsx (1)
8-30: 🛠️ Refactor suggestion
Maintain consistency with other icons.
The CloseIcon has inconsistencies with other icons in the set:
- Different default dimensions (24x25 vs 20x21)
- Different default color (object.low vs object.high)
Consider standardizing these values across all icons for better maintainability.
const CloseIcon: FC<SvgIconProps> = ({ isActive = false, color, - width = '24', - height = '25', + width = '20', + height = '21', + 'aria-label': ariaLabel = 'Close', + role = 'img', ...props }) => { const { colors } = useTheme() - const defaultColor = color ?? colors.object.low + const defaultColor = color ?? colors.object.high const svgColor = isActive ? colors.primary.high : defaultColor return ( <Svg width={width} height={height} - viewBox="0 0 24 25" + viewBox="0 0 20 21" color={svgColor} + role={role} + aria-label={ariaLabel} {...props} >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const CloseIcon: FC<SvgIconProps> = ({ isActive = false, color, width = '20', height = '21', 'aria-label': ariaLabel = 'Close', role = 'img', ...props }) => { const { colors } = useTheme() const defaultColor = color ?? colors.object.high const svgColor = isActive ? colors.primary.high : defaultColor return ( <Svg width={width} height={height} viewBox="0 0 20 21" color={svgColor} role={role} aria-label={ariaLabel} {...props} > <Path fillRule="evenodd" clipRule="evenodd" d="M11.9943 14.0238L17.6486 19.6785C17.8371 19.8606 18.0896 19.9613 18.3516 19.959C18.6137 19.9568 18.8644 19.8516 19.0497 19.6663C19.235 19.481 19.3402 19.2303 19.3424 18.9682C19.3447 18.7061 19.244 18.4536 19.0619 18.2651L13.4076 12.6104L19.0619 6.95573C19.244 6.76721 19.3447 6.51471 19.3424 6.25262C19.3402 5.99053 19.235 5.73982 19.0497 5.55449C18.8644 5.36916 18.6137 5.26403 18.3516 5.26176C18.0896 5.25948 17.8371 5.36023 17.6486 5.54231L11.9943 11.197L6.34004 5.54231C6.15068 5.36473 5.89967 5.26779 5.64011 5.27201C5.38055 5.27622 5.13282 5.38126 4.94932 5.5649C4.76583 5.74854 4.66097 5.99636 4.65694 6.25594C4.65291 6.51552 4.75002 6.76649 4.92772 6.95573L10.581 12.6104L4.92672 18.2651C4.83126 18.3573 4.75511 18.4676 4.70273 18.5895C4.65035 18.7115 4.62277 18.8427 4.62162 18.9754C4.62047 19.1081 4.64576 19.2397 4.69601 19.3626C4.74627 19.4854 4.82049 19.597 4.91433 19.6909C5.00818 19.7847 5.11978 19.859 5.24262 19.9092C5.36545 19.9595 5.49707 19.9848 5.62978 19.9836C5.7625 19.9825 5.89366 19.9549 6.0156 19.9025C6.13755 19.8501 6.24784 19.774 6.34004 19.6785L11.9943 14.0238Z" fill="currentColor" /> </Svg> ) }
anicioalexandre
force-pushed
the
BA-2279-mobile-settings-menu
branch
from
7617cf8 to
a7a6312
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/design-system/package.json (1)
17-18: Build Automation Enhancement with New "prepare" Script
The addition of the"prepare": "pnpm run build"script alongside"prepublishOnly": "pnpm run build"ensures that the package is built automatically before publishing. This improves the reliability of the build lifecycle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/authentication/package.json(1 hunks)packages/components/package.json(2 hunks)packages/config/package.json(1 hunks)packages/design-system/package.json(2 hunks)packages/eslint-plugin/package.json(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/package.json(1 hunks)packages/test/package.json(1 hunks)packages/utils/package.json(1 hunks)packages/wagtail/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/eslint-plugin/package.json
- packages/provider/package.json
- packages/config/package.json
- packages/graphql/package.json
- packages/test/package.json
- packages/authentication/package.json
- packages/utils/package.json
- packages/wagtail/package.json
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Packages
- GitHub Check: Unit Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (7)
packages/design-system/package.json (3)
59-89: Explicit Dependency Versioning for Improved Consistency
The dependency versions (e.g.,"@emotion/cache": "11.11.0","@emotion/react": "11.11.4","@mui/material": "5.15.19", etc.) have been updated from catalog references to fixed versions. This change promotes reproducible builds and clearer version control. Please verify that the fixed versions remain compatible with the rest of the codebase.
92-93: Pinned Peer Dependencies Update
The peer dependencies forreactandreact-domhave been updated to"18.3.1". This explicit pinning helps enforce compatibility across consuming projects.
96-131: Reproducible Dev Environment via Updated Dev Dependencies
The modifications in thedevDependenciessection—such as updating"@babel/preset-env","@babel/preset-react", and"typescript"to fixed versions—ensure a more predictable and stable development environment. It’s advisable to run integration tests after the update to catch any unforeseen incompatibilities introduced by these version bumps.packages/components/package.json (4)
26-27: Addition of "prepare" Script in Build Lifecycle
Similar to the design-system package, adding"prepare": "pnpm run build"complements the existing"prepublishOnly": "pnpm run build"script. This ensures that the build step is triggered in all necessary lifecycle events, reducing the risk of unbuilt artifacts.
70-101: Consistency in Dependency Versioning
The dependency block now uses specific version numbers for packages such as@emotion/cache,@mui/material, and others (e.g.,"next": "14.3.0-canary.24"). This explicit versioning aligns with the broader dependency management strategy and aids in ensuring consistency. Please verify that these stringent versions don’t conflict with any transitive dependencies.
108-109: Update of Peer Dependencies for React Ecosystem
Updating peer dependencies to"react": "18.3.1"and"react-dom": "18.3.1"helps standardize the React versions across the codebase. This explicit pinning is important to avoid version mismatches, especially in complex monorepo environments.
112-178: Enhanced Dev Dependency Stability for Reliable Builds and Testing
The comprehensive update in thedevDependenciessection—including changes for Babel presets, Storybook, Cypress, Jest, and TypeScript—is crucial for maintaining a reproducible development environment. These updates should help mitigate issues arising from implicit version behaviors. Ensure that the build and test pipelines are run post-update to validate compatibility.
| height: height + 4, | ||
| justifyContent: 'center', | ||
| overflow: 'hidden', | ||
| width: width + 4, |
There was a problem hiding this comment.
why the + 4 here ?
There was a problem hiding this comment.
@Ronan-Fernandes there is a border outside the avatar itself with some spacement between it
anicioalexandre
force-pushed
the
BA-2279-mobile-settings-menu
branch
from
a7a6312 to
42451aa
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/design-system/components/native/icons/LogoutIcon/index.tsx (3)
8-19: Consider adding JSDoc comments to document the props.The component is well-structured with proper typing, but adding JSDoc comments would improve documentation and provide better IDE hints for developers using this component.
+/** + * LogoutIcon component to display a logout icon SVG + * @param isActive - Whether the icon is in active state + * @param color - Custom color to override theme color + * @param width - Width of the SVG + * @param height - Height of the SVG + */ const LogoutIcon: FC<SvgIconProps> = ({ isActive = false, color, width = '18', height = '17', ...props }) => {
21-43: Add accessibility attributes to the SVG.SVG icons should include proper accessibility attributes to ensure they're usable with screen readers.
- return ( - <Svg width={width} height={height} viewBox="0 0 18 17" color={svgColor} fill="none" {...props}> + return ( + <Svg + width={width} + height={height} + viewBox="0 0 18 17" + color={svgColor} + fill="none" + role="img" + aria-label="Logout" + {...props} + >
45-47: Consider wrapping with React.memo for performance optimization.For icons that may be rendered frequently, using React.memo could prevent unnecessary re-renders.
} -export default LogoutIcon +export default React.memo(LogoutIcon)Remember to add the corresponding import:
-import { FC } from 'react' +import React, { FC } from 'react'packages/design-system/components/native/icons/ChevronIcon/index.tsx (2)
29-43: Consider optimizing the SVG path.The SVG path is quite detailed. For mobile performance, you might want to verify if this path can be optimized further without visual degradation using tools like SVGO.
46-47: Consider adding JSDoc documentation.Adding JSDoc comments for the component would improve code maintainability and help other developers understand the component's purpose and usage more quickly.
+/** + * ChevronIcon component that renders a directional chevron. + * Can be rotated to point in different directions (left, right, up, down). + * Supports active state styling through the theme. + */ const ChevronIcon: FC<ChevronIconProps> = ({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (34)
packages/components/CHANGELOG.md(1 hunks)packages/components/baseapp-frontend-imports.d.ts(1 hunks)packages/components/modules/profiles/native/ProfileSettingsComponent/index.tsx(1 hunks)packages/components/modules/profiles/native/ProfileSettingsComponent/styles.ts(0 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/components/native/appbars/AppBar/index.tsx(1 hunks)packages/design-system/components/native/appbars/AppBar/styles.ts(1 hunks)packages/design-system/components/native/appbars/AppBar/types.ts(1 hunks)packages/design-system/components/native/avatars/ClickableAvatar/index.tsx(1 hunks)packages/design-system/components/native/avatars/ClickableAvatar/styles.ts(1 hunks)packages/design-system/components/native/avatars/ClickableAvatar/types.ts(1 hunks)packages/design-system/components/native/avatars/index.ts(1 hunks)packages/design-system/components/native/icons/BellIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/BlockIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/ChevronIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/ChevronIcon/types.ts(1 hunks)packages/design-system/components/native/icons/CloseIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/LogoutIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/ProfileSettingsIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/SecurityIcon/index.tsx(1 hunks)packages/design-system/components/native/icons/index.ts(1 hunks)packages/design-system/components/native/views/PageViewWithHeader/index.tsx(1 hunks)packages/design-system/components/native/views/View/index.tsx(1 hunks)packages/design-system/layouts/native/AppbarNavigationLayout/index.tsx(1 hunks)packages/design-system/layouts/native/AppbarNavigationLayout/styles.ts(1 hunks)packages/design-system/layouts/native/AppbarNavigationLayout/types.ts(1 hunks)packages/design-system/layouts/native/BottomNavigationLayout/index.tsx(1 hunks)packages/design-system/layouts/native/BottomNavigationLayout/types.ts(1 hunks)packages/design-system/layouts/native/index.ts(1 hunks)packages/design-system/package.json(2 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- packages/components/modules/profiles/native/ProfileSettingsComponent/styles.ts
✅ Files skipped from review due to trivial changes (2)
- packages/components/package.json
- packages/wagtail/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (25)
- packages/design-system/components/native/views/View/index.tsx
- packages/design-system/layouts/native/AppbarNavigationLayout/types.ts
- packages/design-system/layouts/native/AppbarNavigationLayout/styles.ts
- packages/components/baseapp-frontend-imports.d.ts
- packages/design-system/components/native/views/PageViewWithHeader/index.tsx
- packages/design-system/components/native/appbars/AppBar/styles.ts
- packages/design-system/components/native/icons/SecurityIcon/index.tsx
- packages/design-system/components/native/avatars/index.ts
- packages/design-system/components/native/icons/ChevronIcon/types.ts
- packages/design-system/package.json
- packages/design-system/components/native/avatars/ClickableAvatar/styles.ts
- packages/design-system/layouts/native/BottomNavigationLayout/types.ts
- packages/design-system/components/native/avatars/ClickableAvatar/index.tsx
- packages/design-system/components/native/icons/CloseIcon/index.tsx
- packages/design-system/components/native/icons/BlockIcon/index.tsx
- packages/design-system/components/native/icons/ProfileSettingsIcon/index.tsx
- packages/design-system/components/native/icons/index.ts
- packages/design-system/layouts/native/BottomNavigationLayout/index.tsx
- packages/components/modules/profiles/native/ProfileSettingsComponent/index.tsx
- packages/design-system/components/native/avatars/ClickableAvatar/types.ts
- packages/wagtail/package.json
- packages/design-system/layouts/native/AppbarNavigationLayout/index.tsx
- packages/design-system/layouts/native/index.ts
- pnpm-workspace.yaml
- packages/design-system/components/native/icons/BellIcon/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (10)
packages/design-system/components/native/icons/LogoutIcon/index.tsx (1)
1-47: The LogoutIcon implementation looks good and aligns with the PR objectives.The implementation follows the design system patterns and correctly handles theming and props. The icon represents a logout action which fits with the requirements for the settings menu in the mobile application.
packages/components/CHANGELOG.md (1)
3-10: Changelog entry looks good!The changelog properly documents the architectural change from component-specific AppBar to the more reusable
AppbarNavigationLayout. This aligns well with the PR objective of implementing a mobile settings menu.packages/design-system/components/native/appbars/AppBar/types.ts (1)
1-11: Well-structured type definitions for the AppBar redesignThe updated
AppBarPropstype with the new navigation callbacks (onBack,onClose) and custom icon options significantly improves component flexibility. This aligns perfectly with the settings menu implementation requirements.packages/design-system/CHANGELOG.md (1)
3-13: Comprehensive changelog with clear feature descriptionsThe changelog entry properly documents all the key additions needed for the settings menu implementation, including the AppBar redesign, new icons, ClickableAvatar component, and navigation layouts.
packages/design-system/components/native/appbars/AppBar/index.tsx (1)
1-42: Excellent AppBar redesign with improved navigation capabilitiesThe AppBar component has been significantly improved with:
- More intuitive navigation controls through dedicated back/close buttons
- Better customization options with replaceable icons
- Cleaner layout structure using View components
- Clear separation of concerns with button containers
This implementation perfectly supports the settings menu navigation requirements specified in the PR objectives.
packages/design-system/components/native/icons/ChevronIcon/index.tsx (5)
1-7: Good structure and clean imports.The imports are well-organized with clear separation between external dependencies and internal modules. The typed imports provide good type safety.
8-16: Well-defined component interface with sensible defaults.The component is properly typed using FC with ChevronIconProps. Default values are provided for most props, making the component flexible yet easy to use with minimal configuration.
17-21: Good use of theming system.The component correctly uses the theme provider to access colors and handles both custom colors and active state appropriately. This ensures consistent styling across the application.
22-27: Clean direction mapping implementation.The rotation mapping is well-structured and supports all four directions (right, left, down, up) with appropriate rotation values.
29-43: Verify SVG dimensions and viewBox consistency.The default height (25) differs from the width (24), but the viewBox is "0 0 24 25". While this is likely intentional for this particular icon's aspect ratio, ensure this is consistent with your design specifications.
Acceptance Criteria
** If the profile doesn't have a username, then center the name.
** For now these options will not redirect anywhere, except for the Profile Settings which is already implemented.
** The log out option should be in a separate group called "Actions", while the other 4 options should be grouped in "Settings".
Summary by CodeRabbit
New Features
Refactor
Chores