Skip to content

[Building Sync-ups Tutorial] fixes in "Editing and deleting a sync-up" chapter #3578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

FredericRuaudel
Copy link
Contributor

Hi!

Following my last PR, I'm currently reviewing the "Building Sync-ups" tutorial and so far, there is a lot of changes so I've splitted up into one PR per chapter to ease the review of them, continuing with the "Editing and deleting a sync-up" chapter here…

Changes Made:

  • Replaced the deprecated .cornerRadius() method with the newer .clipShape(.rect(cornerRadius:)) modifier to ensure compatibility with the latest SwiftUI standards.
  • Updated the delete button to use a .destructive role instead of .foregroundColor(.red) for better semantics.
  • Fix compilation errors by using withLock to ensure safe updates to the shared syncUp state during editing.
  • Add a step in SyncUpDetail tutorial to utilize a type-safe .syncUps key when updating the parent list after deletion confirmation action.
  • Make some slight changes in the text of the tutorial documentation due to the removal of outdated commentary regarding delegate actions.

Update view code files to use the newer `.clipShape(.rect(cornerRadius:))` modifier instead of the deprecated `.cornerRadius()` method
… role

- Replace deprecated `.foregroundColor()` with `.foregroundStyle()`
- Add `.destructive` role to delete button
- Remove explicit `.foregroundColor(.red)` styling for the delete button
- Replace `.accentColor` with `Color.accentColor` in foregroundStyle modifier for tutorial code files to fix compilation error "type 'ShapeStyle' has no member 'accentColor'"
Use `withLock` to safely update the shared `syncUp` state when editing a sync-up in the tutorial code files
Refactor SyncUpDetail reducer to:
- Use type-safe `.syncUps` key for shared state
- Implement thread-safe state mutation with `withLock`
Modify tutorial documentation to:
- Simplify explanation of `.run` effect
- Remove unnecessary commentary about sending delegate action which are no longer done with the usage of `@Shared`
Minor code cleanup by removing an extra blank line in the SyncUpDetail reducer that was causing weird diff in the tutorial
Comment on lines 30 to 31
@Shared(.fileStorage(.syncUps)) var syncUps: IdentifiedArrayOf<SyncUp> = []
syncUps.remove(id: state.syncUp.id)
return .none
// @Shared(.fileStorage(.syncUps)) var syncUps: IdentifiedArrayOf<SyncUp> = []
@Shared(.syncUps) var syncUps
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📓 Here I've added a new step to suggest the usage of the typesafe shared key instead of the "basic" .fileStorage syntax. And due to that, as with one of my previous PR, the next diffs will be weird due to the shift of the files numbering.

Comment on lines 271 to 277
@Step {
We can also use the type safe key `.syncUps` we have created earlier to ensure the
type and key are correct and we get a more consise declaration as a bonus.

@Code(name: "SyncUpDetail.swift", file: EditingAndDeletingSyncUp-02-code-0011.swift)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

📓 Here is the new step to suggest the usage of the typesafe key.

Comment on lines -311 to +321
``ComposableArchitecture/Send`` for sending actions back into the system.
``ComposableArchitecture/Send`` for sending actions back into the system if needed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📓 Here I've added "if needed" because the run action has only the call to the dismiss dependency while before there was also the sending of the delegate action. So now, the send is not really needed, hence the update…

Copy link
Member

@stephencelis stephencelis left a comment

Choose a reason for hiding this comment

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

Thanks!

@stephencelis stephencelis merged commit 7953e49 into pointfreeco:main Feb 17, 2025
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.

2 participants