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

demo: change removed property limitVideoWidth to videoResolutionLimit #1345

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

Florent-Bouisset
Copy link
Collaborator

Property limitVideoWidth has been removed in the API for v4.0.0 but the changes was not reflected in the demo.
This PR change this obsolete property by the new videoResolutionLimit in the demo.

@Florent-Bouisset Florent-Bouisset added Demo Relative to the RxPlayer's demo page v4.0.0 Candidate PR/issue for a v4.0.0 release labels Jan 2, 2024
@Florent-Bouisset Florent-Bouisset force-pushed the demo/fix-obsolete-property-limitVideoWidth branch from 9b058fa to ec3e8ac Compare January 2, 2024 16:23
@Florent-Bouisset Florent-Bouisset force-pushed the demo/fix-obsolete-property-limitVideoWidth branch from ec3e8ac to 5836e55 Compare January 3, 2024 15:58
case "videoElement":
videoResolutionLimitDescMsg = "The loaded video Representation will be throttled according to the given videoElement’s dimensions.";
break;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should have a column limit to the demo.
We have one for the source but it does not seem to be enabled for the demo code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Apparently there is one but with the options"ignoreStrings": true that ignores lines that contains a string
I'm ok to remove this, but it should be done in a separate PR as there will be probably a lot of changes when correcting all warnings

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK I agree

@@ -23,7 +26,7 @@ const defaultOptionsValues = {
},
onCodecSwitch: "continue",
},
};
} satisfies { player: IConstructorOptions, loadVideo: ILoadVideoOptions };
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@peaBerberian
Copy link
Collaborator

The code looks good to me, but I would like to know what you think about adding a max column length to the demo for readability purposes

@peaBerberian peaBerberian merged commit 9b00ad2 into next-v4 Jan 3, 2024
4 checks passed
@peaBerberian peaBerberian added this to the 4.0.0-rc.1 milestone Jan 4, 2024
peaBerberian added a commit that referenced this pull request Jan 11, 2024
…mitVideoWidth

demo: change removed property limitVideoWidth to videoResolutionLimit
peaBerberian added a commit that referenced this pull request Jan 11, 2024
…mitVideoWidth

demo: change removed property limitVideoWidth to videoResolutionLimit
peaBerberian added a commit that referenced this pull request Jan 15, 2024
…mitVideoWidth

demo: change removed property limitVideoWidth to videoResolutionLimit
peaBerberian added a commit that referenced this pull request Jan 23, 2024
…mitVideoWidth

demo: change removed property limitVideoWidth to videoResolutionLimit
@peaBerberian peaBerberian deleted the demo/fix-obsolete-property-limitVideoWidth branch February 7, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Demo Relative to the RxPlayer's demo page v4.0.0 Candidate PR/issue for a v4.0.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants