-
Notifications
You must be signed in to change notification settings - Fork 2
feat: create component - navigation bar #631
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
base: develop
Are you sure you want to change the base?
Conversation
bbb8afb to
e59beae
Compare
🟢 Netlify deploy for commit 62e1a0b succeededDeploy preview: https://69120d3bff9852fc28150744--ouds-android.netlify.app |
504e798 to
6565f3f
Compare
37c8ab3 to
b39e900
Compare
b39e900 to
efad5b1
Compare
efad5b1 to
19bd864
Compare
d50b3c6 to
3a334c9
Compare
3a334c9 to
f2766ea
Compare
3cfc6cc to
d5922e5
Compare
# Conflicts: # gradle/libs.versions.toml # Conflicts: # gradle/libs.versions.toml
# Conflicts: # gradle/libs.versions.toml
65c2b82 to
c9385c1
Compare
| app/src/main/res/drawable/ic_palette.xml | ||
| app/src/main/res/drawable/ic_settings.xml | ||
| app/src/main/res/drawable/ic_sms_message.xml | ||
| app/src/main/res/drawable/ic_shop_store.xml |
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.
A detail, ic_shop_store should be placed before ic_sms_message.
| * OudsNavigationBar should contain three to five [OudsNavigationBarItem], each representing a singular destination. | ||
| * | ||
| * OudsNavigationBar default appearance is opaque but, if you need a **translucent blurred navigation bar** (supported from Android 13) as specified on OUDS design | ||
| * side, you can implement it in your app with the help of [Haze](https://chrisbanes.github.io/haze/latest/) library. To do this, use OudsNavigationBar with | ||
| * [translucent] parameter set to true and follow these steps: | ||
| * 1. Add Haze dependency | ||
| * 2. Follow Haze basic usage instructions: | ||
| * - Define Haze state in the screen containing the navigation bar: `val hazeState = rememberHazeState()` | ||
| * - Use `hazeEffect` Modifier on OudsNavigationBar providing OUDS blur radius: `Modifier.hazeEffect(state = hazeState, style = HazeStyle(tint = null, blurRadius = OudsTheme.navigationBarBlur.dp)),` | ||
| * - Apply `hazeSource` Modifier on the content that scrolls behind the navigation bar: `Modifier.hazeSource(state = hazeState)` | ||
| * 3. As your screen content needs to scroll behind the navigation bar, you'll probably need to adjust paddings and to add a spacer at the end of the screen | ||
| * content that will have the height of OudsNavigationBar. For this, please use `OudsNavigationBarHeight` constant. |
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.
| * OudsNavigationBar should contain three to five [OudsNavigationBarItem], each representing a singular destination. | |
| * | |
| * OudsNavigationBar default appearance is opaque but, if you need a **translucent blurred navigation bar** (supported from Android 13) as specified on OUDS design | |
| * side, you can implement it in your app with the help of [Haze](https://chrisbanes.github.io/haze/latest/) library. To do this, use OudsNavigationBar with | |
| * [translucent] parameter set to true and follow these steps: | |
| * 1. Add Haze dependency | |
| * 2. Follow Haze basic usage instructions: | |
| * - Define Haze state in the screen containing the navigation bar: `val hazeState = rememberHazeState()` | |
| * - Use `hazeEffect` Modifier on OudsNavigationBar providing OUDS blur radius: `Modifier.hazeEffect(state = hazeState, style = HazeStyle(tint = null, blurRadius = OudsTheme.navigationBarBlur.dp)),` | |
| * - Apply `hazeSource` Modifier on the content that scrolls behind the navigation bar: `Modifier.hazeSource(state = hazeState)` | |
| * 3. As your screen content needs to scroll behind the navigation bar, you'll probably need to adjust paddings and to add a spacer at the end of the screen | |
| * content that will have the height of OudsNavigationBar. For this, please use `OudsNavigationBarHeight` constant. | |
| * [OudsNavigationBar] should contain three to five [OudsNavigationBarItem], each representing a singular destination. | |
| * | |
| * [OudsNavigationBar] default appearance is opaque but, if you need a **translucent blurred navigation bar** (supported from Android 13) as specified on OUDS design | |
| * side, you can implement it in your app with the help of [Haze](https://chrisbanes.github.io/haze/latest/) library. To do this, use [OudsNavigationBar] with | |
| * [translucent] parameter set to true and follow these steps: | |
| * 1. Add Haze dependency | |
| * 2. Follow Haze basic usage instructions: | |
| * - Define Haze state in the screen containing the navigation bar: `val hazeState = rememberHazeState()` | |
| * - Use `hazeEffect` Modifier on [OudsNavigationBar] providing OUDS blur radius: `Modifier.hazeEffect(state = hazeState, style = HazeStyle(tint = null, blurRadius = OudsTheme.navigationBarBlur.dp)),` | |
| * - Apply `hazeSource` Modifier on the content that scrolls behind the navigation bar: `Modifier.hazeSource(state = hazeState)` | |
| * 3. As your screen content needs to scroll behind the navigation bar, you'll probably need to adjust paddings and to add a spacer at the end of the screen | |
| * content that will have the height of [OudsNavigationBar]. For this, please use [OudsNavigationBarHeight] constant. |
| with(OudsTheme.componentsTokens.bar) { | ||
| NavigationBar( | ||
| modifier = modifier | ||
| .focusProperties { canFocus = false } |
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.
Do you remember why we need this line?
| NavigationBar( | ||
| modifier = modifier | ||
| .focusProperties { canFocus = false } | ||
| .drawBehind { |
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.
drawLine draws the line half inside and half outside the navigation bar, although it's drawn inside in Figma. To fix this we should draw the line like this:
val topBorderWidth = 1.dp.toPx()
drawLine(
color = Color.Red,
start = Offset(x = 0f, y = topBorderWidth / 2f),
end = Offset(x = size.width, y = topBorderWidth / 2f),
strokeWidth = topBorderWidth
)However, the top border is drawn behind the navigation bar, which partially or completely hides the top border (depending if it is translucent or not). But I could not find a way to fix this. Using drawWithContent instead hides the selection indicator (and I suppose this is the reason why you used drawBehind).
| * @sample com.orange.ouds.core.component.samples.OudsNavigationBarSample | ||
| */ | ||
| @Composable | ||
| fun RowScope.OudsNavigationBarItem( |
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.
Should we keep this composable method or should we use an OudsNavigationBarItem class instead, with an items parameter in OudsNavigationBar? Is Maxime OK with the fact that developers will be able to change background color and padding of items for instance?
| var lastItemBadge: ItemBadge by mutableStateOf(lastItemBadge) | ||
|
|
||
| enum class ItemBadge(@StringRes val labelRes: Int) { | ||
| None(R.string.app_components_navigationBar_lastItemBadgeNone_label), |
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.
A detail, but app_components_navigationBar_lastItemBadgeNone_label could be renamed to app_components_navigationBar_itemBadgeNone_label, because maybe one day we will change which item displays the badge and this could apply to every item with no badge.
| bottomSheetContent = { NavigationBarDemoBottomSheetContent(state = state) }, | ||
| codeSnippet = { navigationBarDemoCodeSnippet(state = state, context = context) }, | ||
| demoContent = { NavigationBarDemoContent(state = state) }, | ||
| demoContentPaddingValues = PaddingValues(horizontal = OudsTheme.spaces.fixed.none) |
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.
This is not related to this PR, but other demo screens that uses the demoContentPaddingValues parameter set it to PaddingValues(). We could change these values to PaddingValues(horizontal = OudsTheme.spaces.fixed.none) like you did here because it's better.
| with(state) { | ||
| val itemCountOptions = remember { (MinNavigationBarItemCount..MaxNavigationBarItemCount).toList() } | ||
| CustomizationFilterChips( | ||
| applyTopPadding = true, |
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.
If I am correct there should not be a top padding for the first customization element.
| val isLastItem = index == itemCount - 1 | ||
| val label = context.resources.getString(item.labelRes) | ||
| functionCallArgument(null, "OudsNavigationBarItem") { | ||
| typedArgument("selected", item == NavigationBarItem.Home) |
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.
If we change the selected item in the demo, the code snippet does not reflect it:
typedArgument("selected", index == state.selectedItemId)| topStart = 0.dp, | ||
| topEnd = 0.dp, | ||
| bottomStart = borderRadiusActiveIndicatorCustomTop.value, | ||
| bottomEnd = borderRadiusActiveIndicatorCustomTop.value |
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.
There are also borderRadiusActiveIndicatorCustomBottom and sizeWidthActiveIndicatorCustomBottom tokens. Does that mean that the indicator can be located at the bottom? Or is it something that was experimented during the design phase and tokens will be removed in next version?
Closes #797