-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(UI): Add ad statistics button #6827
Conversation
Incremental code coverage: 29.87% |
contextMenuElements: [ | ||
'loop', | ||
'picture_in_picture', | ||
'statistics', | ||
'ad_statistics', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be sure to add this to docs/tutorials/ui-customization.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doc also lists elements that can be added to different UI areas. See, for example, where it lists picture_in_picture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
* recenter_vr: adds a button that recenter the VR view to the initial view. The button is visible | ||
only if playing a VR content. | ||
* toggle_stereoscopic: adds a button that toggle between monoscopic and stereoscopic. The button | ||
is visible only if playing a VR content. | ||
* ad_statistics: adds a button that displays ad statistics of the video. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should also add ad_statistics
(and fix the capitalization on statistics
) in the listing under #### Replacing the default context menu
, since it looks like your new button can also be added to the context menu.
...Perhaps we should come up with some way of generating this automatically, since it's such an annoying thing to keep in sync. It was simpler to maintain this doc when we didn't have all of these different menus! Well, another time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, maintaining this is a pain, we should change it in the future... :(
No description provided.