Align PR handling of default values #346
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Dec 6, 2023
krocard
changed the title
krocard
requested review from
rolandkakonyi,
matamegger and
zigavehovec
and removed request for
matamegger
1 task
rolandkakonyi
requested changes
Dec 6, 2023
src/components/PlayerView/index.tsx
Outdated
Comment on lines
142
to
143
| if (node && isPictureInPictureRequested) { | ||
| dispatch('setPictureInPicture', node, isPictureInPictureRequested); |
Base automatically changed from
PRN-74/refactor--improve-null-safety
to
development
December 7, 2023 08:36
rolandkakonyi
requested changes
Dec 7, 2023
src/components/PlayerView/index.tsx
Outdated
| @@ -125,21 +125,21 @@ export function PlayerView({ | |||
|
|
|||
| useEffect(() => { | |||
| const node = findNodeHandle(nativeView.current); | |||
| if (node) { | |||
| if (node && isFullscreenRequested != undefined) { | |||
There was a problem hiding this comment.
Please use !== instead as the Check warning indicated!
Suggested change
| if (node && isFullscreenRequested != undefined) { | |
| if (node && isFullscreenRequested !== undefined) { |
There was a problem hiding this comment.
Interesting. Which tool is emitting those warning? If we need to avoid them, should it be part of the CI? (I don't have any warning on my side)
There was a problem hiding this comment.
yarn lint
It already runs on the CI and you can see the warnings only on the Files changed tab.
This should be executed as pre-commit hook if you have husky installed.
There was a problem hiding this comment.
Can we make them errors? #348
Co-authored-by: Roland Kákonyi <[email protected]>
rolandkakonyi
approved these changes
Dec 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem (PRN-92)
Most of the RN SDK relies on the default value from the native side.
The 2 exceptions where fullscreen and PiP, where the default is set in TS.
Nevertheless this is not necessary because the defaults are the same in both native SDK.
Changes
Align the default. Don't send the command if the value is undefined.
📚 Other PRs for this issue