Skip to content

Conversation

@stalkermv
Copy link
Owner

No description provided.

@stalkermv stalkermv requested a review from Copilot May 8, 2025 18:04
@stalkermv stalkermv self-assigned this May 8, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new set of SwiftUI components for creating customizable cells using a custom cell style approach. Key changes include:

  • Introducing the DefaultCustomCellStyle and the CustomCellStyle protocol with a configuration object.
  • Implementing the customizable CustomCell view along with environment extensions for style injection.
  • Updating the package manifest, README, and CI workflows to support the new components.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Sources/CustomCell/CustomCellStyle/DefaultCustomCellStyle.swift Adds the default cell style implementation.
Sources/CustomCell/CustomCellStyle/CustomCellStyleConfiguration.swift Introduces the configuration structure with nested view types.
Sources/CustomCell/CustomCellStyle/CustomCellStyle.swift Defines the protocol for custom cell styles.
Sources/CustomCell/CustomCellStyle/CustomCellStyle+Environment.swift Provides environment keys and extension for cell styles.
Sources/CustomCell/CustomCell.swift Implements the CustomCell view with configurable content.
README.md Documentation update for the newly added component library.
Package.swift Updates package manifest to include new component targets.
LICENSE Adds the MIT license.
.github/workflows/tests.yml CI workflow configuration for building and testing the package.
Comments suppressed due to low confidence (1)

Sources/CustomCell/CustomCellStyle/CustomCellStyleConfiguration.swift:26

  • [nitpick] Using 'Image' as a nested type may cause confusion with SwiftUI's built-in Image type. Consider renaming it to 'CellImage' or a similarly descriptive name to avoid potential ambiguity.
public struct Image: View {

@stalkermv stalkermv requested a review from Copilot May 12, 2025 23:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a collection of new, configurable SwiftUI components—including CustomStepper, CustomSection, CustomCell, and AsyncButton—aimed at providing modular UI elements for SwiftUI projects. Key changes include:

  • Introduction of new customizable components with associated styles and configurations.
  • Updates to the package manifest, README, and CI workflow for integration and testing.
  • Inclusion of preview implementations for visual testing of the components.

Reviewed Changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Sources/CustomStepper/CustomStepper.swift Adds a new customizable stepper control with increment/decrement closures.
Sources/CustomSection/* Introduces new section components and related style protocols.
Sources/CustomCell/* Implements configurable cell components with default styles.
Sources/AsyncButton/* Provides asynchronous button components with execution control and custom button styles.
Package.swift Updates dependencies and target configuration to integrate new components.
README.md Documents the usage and installation of the new UI components.
.github/workflows/tests.yml Adds CI configuration for building and testing the package.

Comment on lines 183 to 197
let elementConfiguration = CustomStepperElementConfiguration(
onIncrement: onIncrement() ?? {},
onDecrement: onDecrement() ?? {}
)

let decrement = (elementStyle ?? style).makeDecrement(configuration: elementConfiguration)
.disabled(onDecrement() == nil)

let increment = (elementStyle ?? style).makeIncrement(configuration: elementConfiguration)
.disabled(onIncrement() == nil)

let configuration = CustomStepperConfiguration(
label: .init(body: .init(label)),
onIncrement: onIncrement() ?? {},
onDecrement: onDecrement() ?? {},
Copy link

Copilot AI May 12, 2025

Choose a reason for hiding this comment

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

onIncrement() and onDecrement() are invoked multiple times (both here and later when checking for nil for the disabled state), which might lead to inconsistent behavior if these closures are not idempotent. Consider storing the results in local constants to ensure a single computation.

Suggested change
let elementConfiguration = CustomStepperElementConfiguration(
onIncrement: onIncrement() ?? {},
onDecrement: onDecrement() ?? {}
)
let decrement = (elementStyle ?? style).makeDecrement(configuration: elementConfiguration)
.disabled(onDecrement() == nil)
let increment = (elementStyle ?? style).makeIncrement(configuration: elementConfiguration)
.disabled(onIncrement() == nil)
let configuration = CustomStepperConfiguration(
label: .init(body: .init(label)),
onIncrement: onIncrement() ?? {},
onDecrement: onDecrement() ?? {},
let resolvedOnIncrement = onIncrement() ?? {}
let resolvedOnDecrement = onDecrement() ?? {}
let elementConfiguration = CustomStepperElementConfiguration(
onIncrement: resolvedOnIncrement,
onDecrement: resolvedOnDecrement
)
let decrement = (elementStyle ?? style).makeDecrement(configuration: elementConfiguration)
.disabled(resolvedOnDecrement == nil)
let increment = (elementStyle ?? style).makeIncrement(configuration: elementConfiguration)
.disabled(resolvedOnIncrement == nil)
let configuration = CustomStepperConfiguration(
label: .init(body: .init(label)),
onIncrement: resolvedOnIncrement,
onDecrement: resolvedOnDecrement,

Copilot uses AI. Check for mistakes.
@stalkermv stalkermv merged commit 7c9c275 into main May 12, 2025
1 check passed
@stalkermv stalkermv deleted the development branch May 12, 2025 23:31
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