Skip to content
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

Add basic ButtonStyle implementation #214

Merged
merged 10 commits into from
Aug 1, 2020
Merged

Add basic ButtonStyle implementation #214

merged 10 commits into from
Aug 1, 2020

Conversation

MaxDesiatov
Copy link
Collaborator

@MaxDesiatov MaxDesiatov commented Jul 25, 2020

This based off the buttonstyles branch by @Outcue.

Initially it didn't work because mounted host views didn't propagate their environment on updates. This is now fixed by adding updateEnvironment function on MountedElement base class and calling it in the initializer. Manual environment updates are no longer needed in makeMounted... factory functions. makeMountedApp is no longer needed at all and MountedApp initializer can be used directly then.

@MaxDesiatov MaxDesiatov added the SwiftUI compatibility Tokamak API differences with SwiftUI label Jul 25, 2020
Comment on lines +14 to +15
//
// Created by Gene Z. Ragan on 07/22/2020.
Copy link
Member

Choose a reason for hiding this comment

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

What’s the format for the header? Are we keeping these lines or removing them?

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 personally don't mind their presence (or absence) at all. They are added by Xcode, while files with no timestamps like that were created in other editors I guess (mine certainly were). I think adding these lines to files that don't currently have them is too much hassle, so if you want consistency it would be probably easier to just remove them everywhere.

Sources/TokamakDOM/Views/Buttons/Button.swift Outdated Show resolved Hide resolved
Sources/TokamakDOM/Views/Buttons/Button.swift Outdated Show resolved Hide resolved
Sources/TokamakDOM/DOMRenderer.swift Outdated Show resolved Hide resolved
@MaxDesiatov
Copy link
Collaborator Author

MaxDesiatov commented Jul 26, 2020

Looks like pointer events are fixed now and isPressed is passed properly, but the actual color changes aren't, which seems similar to problems I saw with environment propagation in #136, working on that...

MaxDesiatov added a commit that referenced this pull request Jul 29, 2020
Implements support for custom scenes by adding the new `MountedScene` type and fixes environment propagation from mounted parent to mounted child elements.

This will unblock #136 and potentially #214. In the former scenes are always completely unmounted and remounted from scratch when root environment changes, which causes descendants to lose all `@State` values. I saw some environment propagation issues in the latter, but not sure if those were caused by the lack of correct scene updates, just wanted to tackle the usual suspects first.

I've also improved reconciler-related doc comments to clarify some of the design desicions and naming.

Resolves #222.
@MaxDesiatov MaxDesiatov marked this pull request as ready for review July 30, 2020 00:15
@MaxDesiatov MaxDesiatov added the bug Something isn't working label Jul 30, 2020
/// Extract all `DynamicProperty` from a type, recursively.
/// This is necessary as a `DynamicProperty` can be nested.
/// `EnvironmentValues` can also be injected at this point.
func dynamicProperties(_ environment: EnvironmentValues,
Copy link
Collaborator Author

@MaxDesiatov MaxDesiatov Jul 30, 2020

Choose a reason for hiding this comment

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

This is moved to MountedElement.swift to maintain just a single extension there.

@MaxDesiatov MaxDesiatov requested review from j-f1 and carson-katri July 30, 2020 00:29
@MaxDesiatov MaxDesiatov requested a review from j-f1 July 30, 2020 12:29
@MaxDesiatov MaxDesiatov merged commit e37d130 into main Aug 1, 2020
@MaxDesiatov MaxDesiatov deleted the buttonstyles branch August 1, 2020 17:46
MaxDesiatov added a commit that referenced this pull request Aug 1, 2020
Merging #214 broke the tests. Also, `DefaultButtonStyle` seems to be trivial enough to be shared through `TokamakCore` between all renderers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SwiftUI compatibility Tokamak API differences with SwiftUI
Development

Successfully merging this pull request may close these issues.

6 participants