-
Notifications
You must be signed in to change notification settings - Fork 116
[V2 Appbar] Added Tooltips to AppBar #790
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
Conversation
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.
Pull Request Overview
This PR adds tooltip support to the V2 AppBar by showing full title, subtitle, or navigation icon content on long press.
- Introduced a
clickableWithTooltip
modifier andTooltip
composable for displaying tooltips. - Updated
AppBar
implementation to wrap title, subtitle, and navigation icon with tooltip support. - Extended
AppBarTokens
with tooltip-related token APIs and updated the demo activity and string resources.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
fluentui_topappbars/src/main/java/com/microsoft/fluentui/tokenized/AppBar.kt | Adds clickableWithTooltip , Tooltip , and integrates tooltips into AppBar. |
fluentui_core/src/main/java/com/microsoft/fluentui/theme/token/controlTokens/AppBarTokens.kt | Adds TooltipControls class and tooltip token methods. |
FluentUI.Demo/src/main/res/values/strings.xml | Adds enable_tooltips string resource. |
FluentUI.Demo/src/main/java/com/microsoft/fluentuidemo/demos/V2AppBarActivity.kt | Adds toggle for enabling tooltips in the demo and overrides token behavior. |
Comments suppressed due to low confidence (5)
FluentUI.Demo/src/main/res/values/strings.xml:58
- [nitpick] For consistency with other AppBar resource keys, consider renaming this to
app_bar_enable_tooltips
.
<string name="enable_tooltips">Enable Tooltips</string>
fluentui_topappbars/src/main/java/com/microsoft/fluentui/tokenized/AppBar.kt:55
- [nitpick] Consider adding unit or UI tests for
clickableWithTooltip
to verify tooltip display and dismissal behavior.
private fun Modifier.clickableWithTooltip(
fluentui_topappbars/src/main/java/com/microsoft/fluentui/tokenized/AppBar.kt:79
- The call to
rememberRipple
will not compile without the proper import. Please addimport androidx.compose.material.ripple.rememberRipple
.
indication = rememberRipple(color = clickRippleColor)
fluentui_core/src/main/java/com/microsoft/fluentui/theme/token/controlTokens/AppBarTokens.kt:8
- The
Offset
import is unused in this file and can be removed to clean up unused imports.
import androidx.compose.ui.geometry.Offset
FluentUI.Demo/src/main/java/com/microsoft/fluentuidemo/demos/V2AppBarActivity.kt:18
- The
rememberUpdatedState
import is not used in this file and should be removed to reduce clutter.
import androidx.compose.runtime.rememberUpdatedState
fluentui_topappbars/src/main/java/com/microsoft/fluentui/tokenized/AppBar.kt
Outdated
Show resolved
Hide resolved
fluentui_topappbars/src/main/java/com/microsoft/fluentui/tokenized/AppBar.kt
Show resolved
Hide resolved
is there a way possible to selectively import components from a module? |
Just checked. It seems like there is no way to selectively have dependencies from components within the module without importing the entire module. The best we can do is move the shared code within the core module so that all the components can leverage it. |
Problem
V2 AppBar Title gets truncated when text size is large, leading to accessibility problems
Fix
Added tooltip when long holding V2 AppBar Title, Subtitle and Navigation Icon
Screenshots
Pull request checklist
This PR has considered: