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

[WCM] Landscape improvements #969

Merged
merged 7 commits into from
Jul 24, 2023
Merged

[WCM] Landscape improvements #969

merged 7 commits into from
Jul 24, 2023

Conversation

radeknovis
Copy link
Contributor

No description provided.

@radeknovis radeknovis temporarily deployed to internal July 18, 2023 10:50 — with GitHub Actions Inactive
@radeknovis radeknovis temporarily deployed to internal July 18, 2023 11:06 — with GitHub Actions Inactive
}
.transform {
#if os(iOS)
$0.autocapitalization(.none)
Copy link
Contributor

Choose a reason for hiding this comment

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

why autocapitalization is inside transform?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, autocapitalization is not available on macOS, so I had to wrap it in this #if os() and this transform helper method allows to do that inline rather than having the entire view duplicated with if/else.

However I've change the condition to really reflect the problem, and add #if not macOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The transform method is really this simple

func transform(@ViewBuilder _ transform: (Self) -> some View) -> some View {
        transform(self)
    }

@radeknovis radeknovis temporarily deployed to internal July 20, 2023 12:21 — with GitHub Actions Inactive
@radeknovis radeknovis temporarily deployed to internal July 20, 2023 13:04 — with GitHub Actions Inactive
Copy link
Contributor

@flypaper0 flypaper0 left a comment

Choose a reason for hiding this comment

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

lgtm

@radeknovis radeknovis temporarily deployed to internal July 24, 2023 12:57 — with GitHub Actions Inactive
@radeknovis radeknovis merged commit 8e0ee9d into develop Jul 24, 2023
8 checks passed
@radeknovis radeknovis deleted the feat/wcm-landscape-fix branch July 24, 2023 13:44
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