Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only return the casting button when there are devices to cast to. #180

Merged
merged 3 commits into from
Feb 1, 2022

Conversation

dero
Copy link
Collaborator

@dero dero commented Jan 31, 2022

Summary

Only resolve the cast button promise when cast receiver devices are detected.

Fixes #179

Copy link
Member

@beaufortfrancois beaufortfrancois left a comment

Choose a reason for hiding this comment

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

The casting button would not be removed when cast devices are not available anymore sadly.

I believe we should have a displayCastState() function that would simply toggle cast button visibility when CAST_STATE_CHANGED event is fired. WDYT?

@dero
Copy link
Collaborator Author

dero commented Jan 31, 2022

@beaufortfrancois Right. I wasn't sure if dynamically removing the cast button from the once established UI without indicating what's happened would be a good solution from the UX perspective.

But I guess with casting it's a relatively common pattern for the button to only appear when cast targets are available.

Updated in 7cdfcdd. Would that work?

src/index.js Outdated

castButton.setAttribute('aria-label', 'Cast this video');
castButton.appendChild(castCustomElement);
castButton.classList.add(CAST_BUTTON_HIDDEN_CLASSNAME);
Copy link
Member

Choose a reason for hiding this comment

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

This line is not needed as castButton will only be added to the document once promise is resolved right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@beaufortfrancois The button is added to the document as soon as the Google Cast API becomes available after the recent changes – regardless of whether cast devices are available or not.

The button visibility (controlled using the class name) then depends on cast state changes.

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that applyCastState() is called just before resolve(castButton). As applyCastState() controls the button visibility, the fist castButton.classList.add(CAST_BUTTON_HIDDEN_CLASSNAME); would not shown to the user.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Quite right. 🤦‍♂️

Thank you. Removed in 3cc1ef6.

Copy link
Member

@beaufortfrancois beaufortfrancois left a comment

Choose a reason for hiding this comment

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

Thank you for working on this!

@derekherman derekherman merged commit cf09a7e into develop Feb 1, 2022
@derekherman derekherman deleted the fix/hide-casting-button-when-no-cast-target branch February 1, 2022 07:22
@felipebochehin87
Copy link

Looks good to me. Cast button is not displayed when there are no devices to cast to.

no cast button

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide "Cast" button when there are no devices to cast
4 participants