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

Update documentation #96

Merged
merged 4 commits into from
Oct 28, 2022
Merged

Update documentation #96

merged 4 commits into from
Oct 28, 2022

Conversation

KVSRoyal
Copy link
Contributor

@KVSRoyal KVSRoyal commented Oct 26, 2022

  • Prevent infinite load of PlatformTabs. These were trying to use router.isReady which shouldn't be used outside of useEffect (link). This causes a progress bar to appear until the user navigates to a different page and then navigates back (e.g. for the Asset Provide Plugin), so remove that.
  • The validations objects link under the Cross-field Validation section in Content/Assets & Views was broken. Add a link to a relevant validations page.
  • Use reliable local images for the Player logo and single/simple flow example.
  • Update the contributing documentation with the info I gathered while working on this so others don't need to.

Version

Published prerelease version: 0.3.0-next.6

Changelog

🚀 Enhancement

🐛 Bug Fix

📝 Documentation

Authors: 8

@hborawski
Copy link
Contributor

@KVSRoyal when you get a chance, rebase against main, fork build should work after #97

@KVSRoyal KVSRoyal marked this pull request as ready for review October 28, 2022 20:25
@@ -45,7 +45,7 @@ The `id` of the views are used in the navigation section to reference a specific

## Cross-field validation

The other special property of a `view` vs. an `asset` is the addition of a `validation` property on the view. These contains [`validation` objects](../guides/validation) that are used for validations crossing multiple fields, and are ran on user navigation rather than data change.
The other special property of a `view` vs. an `asset` is the addition of a `validation` property on the view. These contain [`validation` objects](../plugins/common-types#validations) that are used for validations crossing multiple fields, and are ran on user navigation rather than data change.
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this should be referencing https://github.com/player-ui/player/blob/main/docs/site/pages/content/schema.mdx#validation rather than validations from a plugin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem like I can do a relative link for this (../content/schema#validation). Is it fine if I use the full link: https://github.com/player-ui/player/blob/main/docs/site/pages/content/schema.mdx#validation?

Copy link
Contributor

Choose a reason for hiding this comment

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

can you just do ./schema#validation since its also in the content folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, sorry. My bad. That also doesn't work.

CONTRIBUTING.md Outdated
* [npm >= 8.19.2](https://docs.npmjs.com/downloading-and-installing-node-js-and-npm)
* [yarn >= 1.22.19](https://yarnpkg.com/)
* [Swift >= 5.2](https://www.swift.org/download/)
* [Android NDK = 19.2.5345600](https://github.com/android/ndk/wiki/Unsupported-Downloads#r19c). You'll need to add `ANDROID_NDK_HOME` to your environment manually.
Copy link
Contributor

Choose a reason for hiding this comment

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

We might need to explicitly call out the incompatibility with NDK version >21

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need bazel 6.x to bump up the android ndk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How's this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I looked around, it looks like it's not supported yet.. there's this PR but it's been up for a while and according to this thread, that will no longer be the solution going forward, and there's supposed to be a rules_android_ndk that supports NDK r25. I don't know if that'll require bazel 6.x atm

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/ahumesky/rules_android_ndk
this specifically says version 25, so i'm assuming 22, 23, 24 don't work lol

@@ -101,10 +101,6 @@ const PlatformTabs = (props: React.PropsWithChildren<unknown>) => {
CodeTabsNameMap.has(c.props.mdxType.toLowerCase())
);

if (!router.isReady) {
Copy link
Member

@KetanReddy KetanReddy Oct 28, 2022

Choose a reason for hiding this comment

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

Good catch! I am trying to remember the reason for including this but it looks like its not needed anymore.

@hborawski hborawski merged commit 208d499 into player-ui:main Oct 28, 2022
@KVSRoyal KVSRoyal deleted the docs-update branch October 28, 2022 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants