Skip to content

Add tvOS platform to RTDB #1259

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 24, 2021
Merged

Add tvOS platform to RTDB #1259

merged 2 commits into from
Aug 24, 2021

Conversation

@judyliu14
Copy link
Contributor

@judyliu14 judyliu14 commented Aug 23, 2021 •
edited
Loading

TODO:

  • Fix navigation formatting issue after creating a new post

@google-cla google-cla bot added the cla: yes label Aug 23, 2021
@ncooke3
Copy link
Member

ncooke3 commented Aug 24, 2021 •
edited
Loading

I tested the latest commit on Xcode 13's latest beta and the layout issue seems to have been resolved. I looked at the tvOS beta release notes and this note may have addressed the issue.

@judyliu14 judyliu14 requested a review from ncooke3 August 24, 2021 15:27
@judyliu14
Copy link
Contributor Author

judyliu14 commented Aug 24, 2021

I tested the latest commit on Xcode 13's latest beta and the layout issue seems to have been resolved. I looked at the tvOS beta release notes and this note may have addressed the issue.

That sounds good, this means it should work properly with the latest xcode version.

ncooke3
ncooke3 reviewed Aug 24, 2021
View reviewed changes
database/DatabaseExampleSwiftUI/DatabaseExample/Shared/Views/NewPostsView.swift
Comment on lines +27 to +31
Button(action: {
isPresented = false
}) {
Image(systemName: "chevron.left")
}
Copy link
Member

@ncooke3 ncooke3 Aug 24, 2021

Choose a reason for hiding this comment

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

Double checking, was this new Button view intended to appear on all platforms or just tvOS?

Copy link
Contributor Author

@judyliu14 judyliu14 Aug 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The new button appears on all platforms. The reason why I added it was because tvOS needed a dismiss button to return to the previous page, and I think it would be nice to have for other platforms as well.

ncooke3
ncooke3 approved these changes Aug 24, 2021
View reviewed changes
Copy link
Member

@ncooke3 ncooke3 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, great work! Left one question above 👆

@judyliu14 judyliu14 merged commit 525b133 into master Aug 24, 2021
@judyliu14 judyliu14 deleted the jl-rtdb-tvos branch August 24, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncooke3 ncooke3 ncooke3 approved these changes

@morganchen12 morganchen12 Awaiting requested review from morganchen12

@maksymmalyhin maksymmalyhin Awaiting requested review from maksymmalyhin

@paulb777 paulb777 Awaiting requested review from paulb777

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@judyliu14 @ncooke3