Skip to content

Conversation

@Cramsden
Copy link
Contributor

📜 Tickets

Jira ticket

💡 Description

Add back equatable to BVC state so that we can do redux diffing. Because ShareTab has a webview and has to be main actor isolated I had to add a nonisolated ID property to equate on.

🎥 Demos

Before After
Demo

📝 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

@Cramsden Cramsden requested a review from a team as a code owner December 10, 2025 17:32
@Cramsden Cramsden changed the title Add back equatable for BVC state Add FXIOS-14379 [Performance] Add back equatable for BVC state Dec 10, 2025
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.

Thanks for fixing the issue with ShareTab Equatable!

// Setting default page as topsites
var newTabPageType: NewTabPage = .topSites
var tabUUID: TabUUID = UUID().uuidString
// TODO: FXIOS- This can't be nonisolated because it is a var but it is only set once.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(ticket number)

import Foundation

protocol ShareTab: Sendable {
nonisolated var tabUUID: TabUUID { get }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, seems a reasonable solution for this weird edge case! 👌

Copy link
Contributor

@lmarceau lmarceau left a comment

Choose a reason for hiding this comment

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

+1 on what Isabella said already!

@Cramsden Cramsden force-pushed the cr/FXIOS-14379_add-back-equatable-to-bvc-state branch from 6094f26 to 9debf5e Compare December 11, 2025 05:10
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.

4 participants