-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix some screens for horizontal view #107
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on enhancing the user interface of various components within the onboarding flow. Key changes include the addition of vertical scrolling capabilities to the Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
app/src/main/java/com/canopas/yourspace/ui/component/DatePicker.kt (1)
54-61
: LGTM: Improved DatePicker layout for better usability.The addition of a scrollable
Box
wrapper for theDatePicker
enhances its usability in horizontal view, addressing the PR objective. The implementation is clean and follows Jetpack Compose best practices.Consider adding a content description to the
Box
for improved accessibility:Box(modifier = Modifier .verticalScroll(rememberScrollState()) - .fillMaxWidth() + .fillMaxWidth(), + contentDescription = stringResource(R.string.date_picker_content_description) ) { DatePicker( state = datePickerState ) }app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/SpaceInfoOnboard.kt (1)
36-36
: LGTM: Vertical scrolling added and modifiers reformatted.The addition of vertical scrolling to the main Column Composable addresses the PR objective of fixing screens for horizontal view. The reformatting of button modifiers improves code readability without changing functionality.
Consider extracting the common modifier properties for both buttons into a separate variable to reduce duplication:
val buttonModifier = Modifier .fillMaxWidth() .padding(horizontal = 16.dp) PrimaryButton( modifier = buttonModifier, // ... other properties ) PrimaryOutlinedButton( modifier = buttonModifier, // ... other properties )Also applies to: 54-56, 66-68
app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/JoinOrCreateSpaceOnboard.kt (1)
78-80
: LGTM: Improved layout and scrolling for JoinSpaceComponent.The changes to use
fillMaxSize()
and add vertical scrolling are appropriate for addressing horizontal view issues. This ensures the component adapts well to different screen orientations and allows scrolling when content exceeds the screen height.Consider wrapping the
verticalScroll
modifier in aweight(1f)
modifier to ensure the column takes up all available space while still allowing scrolling:.weight(1f) .verticalScroll(rememberScrollState())
This change would ensure that the column expands to fill the available space before enabling scrolling, which can improve the layout in some cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- app/src/main/java/com/canopas/yourspace/ui/component/DatePicker.kt (3 hunks)
- app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/CreateSpaceOnboard.kt (2 hunks)
- app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/JoinOrCreateSpaceOnboard.kt (3 hunks)
- app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/SpaceInfoOnboard.kt (4 hunks)
🧰 Additional context used
🔇 Additional comments (5)
app/src/main/java/com/canopas/yourspace/ui/component/DatePicker.kt (1)
3-6
: LGTM: New imports for enhanced layout capabilities.The added imports from
androidx.compose.foundation.layout
are appropriate for implementing a scrollable container for the DatePicker. This change aligns with the PR objective of improving the UI for horizontal view.app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/CreateSpaceOnboard.kt (1)
43-43
: Excellent use ofrememberSaveable
for state preservationThe change from
remember
torememberSaveable
is a great improvement. This modification ensures that thespaceName
state is preserved across configuration changes, such as screen rotations. It directly addresses the issue mentioned in the PR objectives regarding name changes when the device is rotated.Benefits of this change:
- Improved user experience: Users won't lose their input when rotating the device.
- Consistency with Jetpack Compose best practices for handling state that needs to survive configuration changes.
- No negative impact on performance or functionality.
app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/SpaceInfoOnboard.kt (1)
11-12
: LGTM: Necessary imports added for vertical scrolling.The addition of
rememberScrollState
andverticalScroll
imports is appropriate for implementing vertical scrolling in the Composable, which aligns with the PR objective of fixing screens for horizontal view.app/src/main/java/com/canopas/yourspace/ui/flow/onboard/components/JoinOrCreateSpaceOnboard.kt (2)
10-11
: LGTM: Necessary imports for vertical scrolling.The addition of
rememberScrollState
andverticalScroll
imports is appropriate for implementing vertical scrolling, which aligns with the PR objectives of fixing screens for horizontal view.
104-106
: LGTM: Improved button layout.The addition of top padding to the PrimaryButton enhances the visual separation and improves the overall layout. This change contributes to a better user experience, which aligns with the PR objectives of improving the UI for different view orientations.
Changelog
[Quick description and/or mentions to other PRs]
Breaking Changes
New Features
SpaceInfoOnboard
,JoinSpaceComponent
,DatePickerDialog
remeberSaveable
property forCreateSpaceOnboard
Bug Fixes
GroupOnboard.mp4
CreateOrJoinGroupScreen.mp4
GroupNameNameScreen.mp4
Summary by CodeRabbit
Release Notes
New Features
DatePicker
component with vertical scrolling for improved usability when selecting dates.JoinSpaceComponent
andSpaceInfoOnboard
, allowing for better navigation of overflowing content.Improvements
CreateSpaceOnboard
to retainspaceName
across configuration changes.JoinSpaceComponent
andSpaceInfoOnboard
for better visual clarity.These changes enhance user experience and interface interactivity across various components.