Skip to content

Conversation

@PARAIPAN9
Copy link
Collaborator

@PARAIPAN9 PARAIPAN9 commented Nov 3, 2025

📜 Tickets

Jira ticket
Github issue

💡 Description

📝 Checklist

  • I filled in the ticket numbers and a description of my work
  • I updated the PR name to follow our PR naming guidelines
  • I ensured unit tests pass and wrote tests for new code
  • If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
  • If adding telemetry, I read the data stewardship requirements and will request a data review
  • If adding or modifying strings, I read the guidelines and will request a string review from l10n
  • If needed, I updated documentation and added comments to complex code

@PARAIPAN9 PARAIPAN9 requested a review from a team as a code owner November 3, 2025 15:40
@PARAIPAN9 PARAIPAN9 requested a review from dataports November 3, 2025 15:40
@mergify
Copy link
Contributor

mergify bot commented Nov 3, 2025

This pull request has conflicts when rebasing. Could you fix it @PARAIPAN9? 🙏

@PARAIPAN9 PARAIPAN9 removed the request for review from dataports November 3, 2025 15:48
@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Nov 4, 2025

Messages
📖 Project coverage: 37.56%

💪 Quality guardian

2 tests files modified. You're a champion of test coverage! 🚀

🥇 Perfect PR size

Smaller PRs are easier to review. Thanks for making life easy for reviewers! ✨

🦊 BrowserViewController Check

We’re tracking the size of BrowserViewController.swift to keep it healthy.

  • ✨ Change in file size: +20 lines

❌ Per-file test coverage gate

The following changed file(s) are below 35.0% coverage:

File Coverage Required
firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift 30.7% 35.0%

Client.app: Coverage: 37.02

File Coverage
AddressBarState.swift 95.46%
AutofillAccessoryViewButtonItem.swift 88.24%
AccessoryViewProvider.swift 69.39%
AddressToolbarContainer.swift 41.94% ⚠️
ToolbarState.swift 91.69%
BrowserViewController.swift 30.66% ⚠️
LegacyHomepageViewController.swift 36.07% ⚠️
ToolbarAction.swift 100.0%
LegacyTabScrollController.swift 68.2%
BrowserViewController+TabScrollHandlerDelegate.swift 0.0% ⚠️
BrowserCoordinator.swift 67.69%

ToolbarKit: Coverage: 62.78

File Coverage
LocationContainer.swift 76.92%
LocationView.swift 60.84%
BrowserAddressToolbar.swift 67.9%

Generated by 🚫 Danger Swift against 6a928da

Copy link
Collaborator

@ih-codes ih-codes left a comment

Choose a reason for hiding this comment

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

Tested locally and seems to be working well! 👍 I'll defer to @thatswinnie since she has more context on the task at hand after reviewing the initial PR.

@mergify
Copy link
Contributor

mergify bot commented Nov 5, 2025

This pull request has conflicts when rebasing. Could you fix it @PARAIPAN9? 🙏

@PARAIPAN9 PARAIPAN9 merged commit c3c05d8 into main Nov 5, 2025
11 checks passed
@PARAIPAN9 PARAIPAN9 deleted the paraipan/fxios-13799-Toolbar-accessory-view-ios26-update-2 branch November 5, 2025 10:55
@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2025

🚀 PR merged to main, targeting version: 145.1

if isEditing {
store.dispatch(ToolbarAction(windowUUID: windowUUID,
actionType: ToolbarActionType.hideKeyboard))
store.dispatchLegacy(
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 dispatchLegacy 🚫 I'll remove it in my next PR, but let's not add new ones please! 🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added it by mistake after solving a conflict. Sorry.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's easy to add them back in since we don't use @available(*, deprecated) 😢

Copy link
Contributor

@lmarceau lmarceau Nov 6, 2025

Choose a reason for hiding this comment

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

I'll add a Danger check for now #30420 so we have an interim solution

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.

6 participants