Base "PublicShareAuth" on "Embedded" instead of on "Application" #1633
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
For legacy reasons the "incall" CSS class, which is used to hide and show the empty content view and the call container as needed, and the "screensharing" CSS class, which is used to adjust the call UI when there are shared screens, were set in the "#talk-sidebar" element of PublicShareAuth. For consistency with the embedded Talk UI now the classes are set in the "#call-container" instead. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This is just a copy of the App code related to guest names; both codes should be refactored and extracted to a common place, but for now copying it is good enough. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
"Embedded" does not provide a method to change the page title, as in general only full apps should do that. However, in the case of the Talk UI in the public share authentication page it makes sense to change the page title; once PublicShareAuth is based on Embedded instead of on Application it will no longer have access to that method, so now PublicShareAuth uses its own copy. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
hen PublicShareAuth was introduced there was no Embedded class, so it had to be based on Application. However, Application should be used only for the main Talk UI and, as its name implies, Embedded should be used for any embedded Talk UI. Some minor adjustments are needed to PublicShareAuth due to the differences between Application and Embedded: unlike in Application, the empty content view is not created by Embedded, so there is no need to destroy it; Embedded does not render the local video view by default, so it needs to be explicitly rendered; there is no fullscreen button for embedded apps, so there is no need to register its handler; and finally, the chat view needs to be explicitly detached when the room is left, as Embedded does not handle UI states. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Now that PublicShareAuth is based in Embedded there is no need to explicitly show the videos and screens when the sidebar is open again, as they will not have been hidden. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
added
3. to review
feature: talk-sidebar ⬅️
Ivansss
approved these changes
Mar 19, 2019
Ivansss
deleted the
base-publicshareauth-on-embedded-instead-of-on-application
branch
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.
This is just a refactoring, so everything should work in the public share authentication page as before.