Skip to content

fix(iOS): icon inside header button is not center aligned on iOS 26#7105

Merged
Rohit3523 merged 4 commits into
developfrom
header-button-fix
Apr 7, 2026
Merged

fix(iOS): icon inside header button is not center aligned on iOS 26#7105
Rohit3523 merged 4 commits into
developfrom
header-button-fix

Conversation

@Rohit3523

@Rohit3523 Rohit3523 commented Apr 6, 2026

Copy link
Copy Markdown
Member

Proposed changes

In #6875, WE added support for glass design in iOS 26 and Icon inside header button is not center aligned and moved to left because of left margin.

Issue(s)

https://rocketchat.atlassian.net/browse/CORE-2065

How to test or reproduce

Screenshots

Before After
Screenshot 2026-04-07 at 3 50 27 AM Screenshot 2026-04-07 at 3 49 59 AM

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

  • Bug Fixes
    • Improved header button positioning for consistent spacing across tablets, Android, and newer iOS versions by centralizing margin logic and resolving previous inconsistencies in left/right button spacing.

@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 80421abc-1030-4eb1-a526-14b24616a2d6

📥 Commits

Reviewing files that changed from the base of the PR and between fddf2e6 and cbe60c6.

📒 Files selected for processing (1)
  • app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: ESLint and Test / run-eslint-and-test
  • GitHub Check: format
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.

Applied to files:

  • app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx
🔇 Additional comments (4)
app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx (4)

2-4: LGTM!

Import changes are minimal and necessary—Platform for version checking and isIOS for the platform condition.


13-21: LGTM!

The getMargin() helper correctly centralizes the platform-aware margin logic. The >= 26 condition properly includes iOS 26, addressing the alignment fix objective.


23-23: LGTM!

Computing the margin once at module load is appropriate since device characteristics are static.


31-37: LGTM!

Styles correctly use the centralized margin value while preserving the existing Android-specific marginRight behavior in the left container.


Walkthrough

Centralized header button margin logic in one component: added platform/version-aware getMargin() and replaced inline isTablet ? 5 : -5 margin expressions to address icon centering on iOS.

Changes

Cohort / File(s) Summary
Header Button Margin Logic Fix
app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx
Added Platform and isIOS imports; introduced getMargin() and computed margin. Replaced inline isTablet ? 5 : -5 with margin for marginLeft/marginRight. margin resolves to 5 for tablets, 0 for iOS when Platform.Version >= 26, and -5 otherwise. Preserved existing marginRight: isAndroid ? 5 : 0 behavior for left layout.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the iOS 26 icon alignment issue being fixed in the HeaderButton component.
Linked Issues check ✅ Passed The code changes directly address the CORE-2065 requirement by adjusting margins to center-align the header button icon on iOS 26.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the header button icon alignment issue; no unrelated modifications were introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Rohit3523 Rohit3523 had a problem deploying to official_android_build April 6, 2026 22:23 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build April 6, 2026 22:23 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build April 6, 2026 22:23 — with GitHub Actions Error
@Rohit3523 Rohit3523 changed the title fix(iOS): native header button is not center aligned in iOS 26 fix(iOS): icon inside header button is not center aligned in iOS 26 Apr 6, 2026
@Rohit3523 Rohit3523 marked this pull request as ready for review April 6, 2026 22:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx`:
- Around line 17-18: The conditional that gates the iOS icon-alignment fix in
HeaderButtonContainer currently uses "Platform.Version > 26" which excludes iOS
26; update the condition in the HeaderButtonContainer where isIOS and
Platform.Version are checked (e.g., the function/logic that computes the icon
margin or "margin" return) to include iOS 26 by using a >= check on
Number(Platform.Version) (i.e., treat versions 26 and above the same) so iOS 26
users receive the fix; ensure the comparison still coerces Platform.Version to a
number as currently done.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dedcca4d-d613-4252-8f57-3d6f2953498c

📥 Commits

Reviewing files that changed from the base of the PR and between 91b2234 and fddf2e6.

📒 Files selected for processing (1)
  • app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
📚 Learning: 2026-03-10T15:21:45.098Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7046
File: app/containers/InAppNotification/NotifierComponent.stories.tsx:46-75
Timestamp: 2026-03-10T15:21:45.098Z
Learning: In `app/containers/InAppNotification/NotifierComponent.tsx` (React Native, Rocket.Chat), `NotifierComponent` is exported as a Redux-connected component via `connect(mapStateToProps)`. The `isMasterDetail` prop is automatically injected from `state.app.isMasterDetail` and does not need to be passed explicitly at call sites or in Storybook stories that use the default (connected) export.

Applied to files:

  • app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx
🔇 Additional comments (2)
app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx (2)

2-4: LGTM!

The imports are correctly extended to include Platform for version detection and isIOS for platform identification.


23-37: LGTM!

The centralized getMargin() approach is cleaner than inline conditionals, and the module-level computation is appropriate since platform and version values are static during runtime.

Comment thread app/containers/Header/components/HeaderButton/HeaderButtonContainer.tsx Outdated
@Rohit3523 Rohit3523 had a problem deploying to official_android_build April 6, 2026 22:45 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build April 6, 2026 22:45 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build April 6, 2026 22:45 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build April 7, 2026 19:16 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to official_android_build April 7, 2026 19:16 — with GitHub Actions Error
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build April 7, 2026 19:16 — with GitHub Actions Error

@OtavioStasiak OtavioStasiak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

@Rohit3523 Rohit3523 had a problem deploying to approve_e2e_testing April 7, 2026 19:50 — with GitHub Actions Failure
@Rohit3523 Rohit3523 had a problem deploying to experimental_ios_build April 7, 2026 19:55 — with GitHub Actions Failure
@Rohit3523 Rohit3523 had a problem deploying to official_ios_build April 7, 2026 19:55 — with GitHub Actions Failure
@Rohit3523 Rohit3523 had a problem deploying to official_android_build April 7, 2026 19:55 — with GitHub Actions Failure
@Rohit3523 Rohit3523 had a problem deploying to experimental_android_build April 7, 2026 19:55 — with GitHub Actions Failure
@Rohit3523 Rohit3523 merged commit 87229ae into develop Apr 7, 2026
6 of 11 checks passed
@Rohit3523 Rohit3523 deleted the header-button-fix branch April 7, 2026 19:57
@Rohit3523 Rohit3523 changed the title fix(iOS): icon inside header button is not center aligned in iOS 26 fix(iOS): icon inside header button is not center aligned on iOS 26 Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants