Skip to content

Refactor FXIOS-15349 [Technical Debt] [Redux] Use @Copyable macro for TabPeekState#34744

Open
hkarmoush wants to merge 1 commit into
mozilla-mobile:mainfrom
hkarmoush:worktree-tabpeek-copyable-macro
Open

Refactor FXIOS-15349 [Technical Debt] [Redux] Use @Copyable macro for TabPeekState#34744
hkarmoush wants to merge 1 commit into
mozilla-mobile:mainfrom
hkarmoush:worktree-tabpeek-copyable-macro

Conversation

@hkarmoush

Copy link
Copy Markdown

📜 Tickets

Jira ticket
Github issue

💡 Description

Migrates TabPeekState (Redux state) to use the @Copyable macro from the ModifiedCopy package, replacing the reducer's manual full-struct reconstruction with chained .copy(property:) calls. This follows the same pattern already used by other Redux states in the codebase (e.g. SearchBarState, HomepageState).

Pure technical-debt refactor — no behavior change, no public API change. showCloseTab is intentionally left out of the .copy() chain since the existing reducer never updates it (always stays at its default true); confirmed via TabPeekState( grep that no call site ever constructs it with a non-default value, so this is behavior-identical to before.

TabPeekStateTests.swift is unmodified.

🎥 Demos

N/A — no UI or behavior change.

📝 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 — note: could not run xcodebuild test locally (no iOS Simulator runtimes installed in this environment); relying on CI to verify TabPeekStateTests (5 existing tests) still pass unmodified
  • N/A — no UI changes
  • N/A — no telemetry changes
  • N/A — no string changes
  • N/A — no documentation changes needed

@PARAIPAN9 PARAIPAN9 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.

Thanks @hkarmoush for your contribution. There is one important change to make in order for the project to build.


@Copyable
struct TabPeekState: ScreenState {
let showAddToBookmarks: Bool

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.

From the build errors I believe we need to add let windowUUID: WindowUUID above let showAddToBookmarks: Bool.

.copy(previewAccessibilityLabel: tabPeekModel.accessiblityLabel)
.copy(screenshot: tabPeekModel.screenshot)
default:
return state

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.

Shouldn't we use here: defaultState(from state:_ )? CC: @ih-codes.

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.

Yes, good catch @PARAIPAN9, it should. 👀 Also on the two guard return lines above. But that can be in a separate PR, since it's not related to adding @Copyable. So I don't think @hkarmoush needs to worry about that here.

@hkarmoush
hkarmoush force-pushed the worktree-tabpeek-copyable-macro branch from 1f4838f to ac60483 Compare July 22, 2026 15:42
@PARAIPAN9

Copy link
Copy Markdown
Contributor

@hkarmoush , everything's ok, we just need to address this to make sure the project builds without errors: #34744 (comment)

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