Skip to content

🎨 Palette: Improve compose FAB long-press accessibility discoverability#119

Closed
Copilot wants to merge 13 commits intomasterfrom
copilot/improve-accessibility-button
Closed

🎨 Palette: Improve compose FAB long-press accessibility discoverability#119
Copilot wants to merge 13 commits intomasterfrom
copilot/improve-accessibility-button

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 29, 2026

  • Review new actionable PR comment and confirm required fix scope
  • Investigate CI/build status via GitHub Actions runs and logs
  • Inspect current FAB long-click wiring in both affected fragments
  • Fix listener regression so long-press works again after account-count changes
  • Run targeted local validation commands
  • Run automated code review and security scan
  • Reply to the new PR comment with commit hash and screenshot

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI changed the title [WIP] Enhance button accessibility with ARIA labels 🎨 Palette: Add tooltip affordance to compose FAB Mar 29, 2026
Copilot AI requested a review from dlukt March 29, 2026 15:47
@dlukt dlukt marked this pull request as ready for review March 29, 2026 15:51
Copilot AI review requested due to automatic review settings March 29, 2026 15:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds tooltip affordance to the icon-only “new post” compose FAB to improve discoverability and align with existing UI patterns that use android:tooltipText.

Changes:

  • Added android:tooltipText="@string/new_post" to the compose FAB in timeline and shared FAB layouts.
  • Minor formatting/indentation normalization in fragment_timeline.xml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
mastodon/src/main/res/layout/fragment_timeline.xml Adds tooltip text to the timeline FAB (and normalizes closing tag indentation).
mastodon/src/main/res/layout/compose_fab.xml Adds tooltip text to the reusable compose FAB layout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 34 to 36
android:contentDescription="@string/new_post"
android:tooltipText="@string/new_post"
android:src="@drawable/ic_fluent_compose_24_filled"/>
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

android:tooltipText is unlikely to appear on long-press here because the FAB has an OnLongClickListener (used for account picking when multiple accounts) which typically suppresses the tooltip when the long-click is consumed. If the goal is long-press discoverability, consider adjusting the FAB long-click handling (or showing the affordance via another mechanism) so the tooltip can still be displayed; otherwise this tooltip will mainly help hover/mouse use cases.

Copilot uses AI. Check for mistakes.
Comment on lines 13 to 16
android:stateListAnimator="@animator/fab_shadow"
android:contentDescription="@string/new_post"
android:tooltipText="@string/new_post"
android:src="@drawable/ic_fluent_compose_24_filled" />
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as in fragment_timeline.xml: this FAB is wired up with an OnLongClickListener in code (account picker when multiple accounts), which commonly prevents tooltips from showing on long-press. If the intent is to improve long-press discoverability, the long-click/tooltip interaction needs to be handled explicitly; otherwise this is primarily a hover tooltip.

Copilot uses AI. Check for mistakes.
@dlukt
Copy link
Copy Markdown
Owner

dlukt commented Mar 29, 2026

@copilot apply changes based on the comments in this thread

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 29, 2026

@copilot apply changes based on the comments in this thread

Applied in commit 778998b (with the functional change in 6d382a8): I removed the FAB tooltip approach and instead added explicit accessibility long-click action labels on the compose FAB where long-press is consumed by account picker. It now exposes “Add or switch accounts” via ACTION_LONG_CLICK when multiple accounts exist, and clears that action label otherwise. Screenshot: https://github.com/user-attachments/assets/8285617d-2cee-4b20-a636-8487ba5bc7b0

Copilot AI changed the title 🎨 Palette: Add tooltip affordance to compose FAB 🎨 Palette: Improve compose FAB long-press accessibility discoverability Mar 29, 2026
@dlukt dlukt requested a review from Copilot March 29, 2026 17:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +153 to +157
if (AccountSessionManager.getInstance().getLoggedInAccounts().size() > 1) {
ViewCompat.replaceAccessibilityAction(fab, AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK, getString(R.string.manage_accounts), null);
} else {
ViewCompat.replaceAccessibilityAction(fab, AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK, null, null);
}
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long-click accessibility label is set only once during view creation. If the user adds/removes accounts (changing getLoggedInAccounts().size()) while this fragment stays alive (e.g., via account management screens and returning), TalkBack can advertise an out-of-date long-press action label that no longer matches what long-press does. Consider extracting this into an updateFabLongClickAccessibilityAction() method and calling it from a lifecycle callback that runs on return/visibility changes (e.g., onResume()/onShown()), so the label always reflects the current account count.

Copilot uses AI. Check for mistakes.
Comment on lines +561 to +565
if (AccountSessionManager.getInstance().getLoggedInAccounts().size() > 1) {
ViewCompat.replaceAccessibilityAction(fab, AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK, getString(R.string.manage_accounts), null);
} else {
ViewCompat.replaceAccessibilityAction(fab, AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK, null, null);
}
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This accessibility action label is computed only at onViewCreated time. If the logged-in account count changes while the fragment remains in the back stack, the ACTION_LONG_CLICK label can become stale (e.g., still announcing “manage accounts” after the user logs out to a single account, even though long-press won’t open the picker). Consider centralizing this logic (e.g., updateFabLongClickAccessibilityAction()) and invoking it from onResume()/onShown() as well, so assistive tech always gets the correct long-press description.

Copilot uses AI. Check for mistakes.
@dlukt
Copy link
Copy Markdown
Owner

dlukt commented Mar 29, 2026

@copilot apply changes based on the comments in this thread

Copilot AI and others added 2 commits March 29, 2026 17:26
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 29, 2026

@copilot apply changes based on the comments in this thread

Applied in 89e27bb (with the lifecycle refresh extraction in acf9ad5). The compose FAB long-click accessibility action is now centralized in updateFabLongClickAccessibilityAction() and refreshed both during setup and when the fragment is shown again, so account-count changes don’t leave stale TalkBack labels. Screenshot: https://github.com/user-attachments/assets/7a07f610-b376-4dcf-b3cf-e8d3d4224410

@dlukt dlukt requested a review from Copilot March 30, 2026 00:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

dlukt and others added 2 commits March 30, 2026 10:55
…stFragment.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ent.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +574 to +581
boolean multipleAccounts = AccountSessionManager.getInstance().getLoggedInAccounts().size() > 1;

// Only expose a long-click action (including to accessibility services)
// when there are multiple accounts to manage.
fab.setLongClickable(multipleAccounts);
if (!multipleAccounts) {
fab.setOnLongClickListener(null);
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateFabLongClickAccessibilityAction() nulls the FAB’s OnLongClickListener when only one account exists, but doesn’t restore it when multipleAccounts becomes true later (e.g., after adding a second account and returning to the fragment). This leaves long-press permanently broken until the view is recreated. Fix by keeping the listener attached and relying on setLongClickable(false), or reattaching fab.setOnLongClickListener(this::onFabLongClick) when multipleAccounts is true.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 688a383. I removed the fab.setOnLongClickListener(null) branch so the long-click listener remains attached, and we rely on fab.setLongClickable(multipleAccounts) plus accessibility-action updates to control exposure. This restores long-press behavior when account count returns to multiple accounts. Screenshot: https://github.com/user-attachments/assets/6ebac5ec-7eee-4818-9052-efcab138219d

…ent.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dlukt/techidon/sessions/2ed82beb-b54c-4425-86d0-1ca04da739dd

Co-authored-by: dlukt <201112286+dlukt@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +570 to +575
private void updateFabLongClickAccessibilityAction() {
if (fab == null)
return;

boolean multipleAccounts = AccountSessionManager.getInstance().getLoggedInAccounts().size() > 1;

Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateFabLongClickAccessibilityAction() duplicates the same logic now present in HomeTabFragment (and partially in other views). To reduce the chance of future drift (e.g., label/resource changes), consider extracting this into a shared helper (e.g., in UiUtils) that takes the target view and label resource.

Copilot uses AI. Check for mistakes.
Comment on lines +704 to +720
fab.setLongClickable(multipleAccounts);

if (multipleAccounts) {
ViewCompat.replaceAccessibilityAction(
fab,
AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK,
getString(R.string.manage_accounts),
null
);
} else {
ViewCompat.replaceAccessibilityAction(
fab,
AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK,
null,
null
);
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateFabLongClickAccessibilityAction() mirrors the same implementation added in BaseStatusListFragment. Consider extracting a shared helper (e.g., UiUtils.updateLongClickActionLabel(view, enabled, @StringRes label)) to avoid duplicated accessibility logic across fragments.

Suggested change
fab.setLongClickable(multipleAccounts);
if (multipleAccounts) {
ViewCompat.replaceAccessibilityAction(
fab,
AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK,
getString(R.string.manage_accounts),
null
);
} else {
ViewCompat.replaceAccessibilityAction(
fab,
AccessibilityNodeInfoCompat.AccessibilityActionCompat.ACTION_LONG_CLICK,
null,
null
);
}
UiUtils.updateLongClickActionLabel(fab, multipleAccounts, R.string.manage_accounts);

Copilot uses AI. Check for mistakes.
dlukt and others added 2 commits March 31, 2026 01:36
…stFragment.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ent.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dlukt dlukt closed this Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants