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

Prevent "use desktop/electron app" error message ...when using that app! #224

Closed
adrienjoly opened this issue Sep 22, 2019 · 6 comments · Fixed by #294
Closed

Prevent "use desktop/electron app" error message ...when using that app! #224

adrienjoly opened this issue Sep 22, 2019 · 6 comments · Fixed by #294

Comments

@adrienjoly
Copy link
Member

adrienjoly commented Sep 22, 2019

Describe the bug

A few years ago, Google Chrome prevented videos from being played without user intervention, when the tab is not focused.

In order to not let new users think that Openwhyd is not able to play tracks sequentially because of that new behavior, I decided to pause the playback whenever the user would switch to another track, so that they can see the following recommendation:

Oops, we could not play this track... Please try with Openwhyd Desktop App 👌

Indeed, the "Openwhyd Desktop App" (downloadable from https://openwhyd.org/download) is able to play tracks sequentially, even in the background.

Unfortunately, that same error message is displayed every time a track fails to be played, for any reason, including from the Openwhyd Desktop App itself! 😬

To Reproduce

  1. Start the Openwhyd Desktop App (you can download the latest versionfrom https://openwhyd.org/download)
  2. Open the following posted track from the App: https://openwhyd.org/c/53035251200c8a63270004b3
  3. Play the track, it should display the error message

Expected behavior

When a track fails to play from the Desktop App, it should displayed another error message. Something like:

Oops, we could not play this track...

But, when failing to play that track from openwhyd.org (in Google Chrome), the error message should be left as it currently is. (i.e. invite the user to install the Desktop App)

Screenshots

Here's what the error looks like:

Capture d’écran 2020-03-15 à 11 10 04

You can find the implementation of that error message in whydPlayer.js

@adrienjoly adrienjoly added the bug label Sep 22, 2019
@adrienjoly adrienjoly added this to 📥 Inbox / ideas in Development via automation Sep 22, 2019
@adrienjoly adrienjoly moved this from 📥 Inbox / ideas to ⚡️To Do Next in Development Sep 24, 2019
@adrienjoly adrienjoly added the hacktoberfest Contribute --> win a t-shirt! label Oct 15, 2019
@adrienjoly adrienjoly added good first issue and removed hacktoberfest Contribute --> win a t-shirt! labels Nov 26, 2019
@compiuta
Copy link
Contributor

@adrienjoly there are 2 issues when trying to test a solution on my local machine.

I have used a solution suggested here electron/electron#2288 (comment)

  1. For some reason when searching for anything on my local install of openwhyd I never seem to get any results. Let me know if this is just on my machine or if you are having the same issue.

Screenshot from 2020-03-19 14-39-15

  1. Is there an easy way to test changes on the electron app locally?

@adrienjoly
Copy link
Member Author

For some reason when searching for anything on my local install of openwhyd I never seem to get any results. Let me know if this is just on my machine or if you are having the same issue.

@compiuta It's expected. Search is provided by a third-party: Algolia, and the Algolia credentials provided in the env-vars-testing of this repository are read-only.

If you want your local search index to be populated after adding tracks, you should create an Algolia account, and set the ALGOLIA_APP_ID and ALGOLIA_API_KEY environment variables accordingly.

@adrienjoly
Copy link
Member Author

Is there an easy way to test changes on the electron app locally?

@compiuta Yes, there is:

  1. run openwhyd locally (e.g. on port 8080)
  2. clone https://github.com/openwhyd/openwhyd-electron on your computer
  3. change its code so it opens http://localhost:8080 instead of https://openwhyd.org

Btw, the openwhyd-electron repository could benefit from more documentation. Feel free to send a Pull Request there if you want to propose some!

Adrien

@adrienjoly
Copy link
Member Author

@compiuta I also wanted to let you know that we already had some code in openwhyd's repository to check if it's running in electron.

A quick search for "electron" in the source code reveals a getBrowserVersionFromUserAgent() function in app/contollers/api/post.js, which detects openwhyd-electron from the user agent.

You can probably re-use a similar logic in the front-end, as the user agent is made available by the browser.

@compiuta
Copy link
Contributor

I will try out your suggestions and get back to you later 👍

@compiuta
Copy link
Contributor

compiuta commented Mar 22, 2020

@adrienjoly I was able to test the electron app locally and a variable that exists in the whydPlayer.js file USING_ELECTRON is actually already detecting if we are in the electron app.

I added it to the if condition let me know if the message is now changing for you when using the electron app or an internet browser.

Development automation moved this from ⚡️To Do Next to ✔️ Done / pending QA Mar 22, 2020
adrienjoly pushed a commit that referenced this issue Mar 22, 2020
…294)

Closes #224 .

## What does this PR do / solve?

Show track error message in electron desktop app without "... Please try with Openwhyd Desktop App"

## Overview of changes

I used a suggested solution from this issue electron/electron#2288 (comment)

By detecting if the user agent contains "electron" it is possible to show a unique message for desktop browsers or the electron app.

## How to test this PR?

See #224 (comment)
adrienjoly pushed a commit that referenced this issue Mar 22, 2020
## [1.30.2](v1.30.1...v1.30.2) (2020-03-22)

### Bug Fixes

* **ui:** Display correct track error message when using electron app ([#294](#294)) ([4f97ccd](4f97ccd)), closes [#224](#224) [/github.com/electron/electron/issues/2288#issuecomment-337858978](https://github.com//github.com/electron/electron/issues/2288/issues/issuecomment-337858978) [/github.com//issues/224#issuecomment-601430390](https://github.com//github.com/openwhyd/openwhyd/issues/224/issues/issuecomment-601430390)
@adrienjoly adrienjoly moved this from ✔️ Done / pending QA to 🌲 In production in Development Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development
  
🌲 In production
Development

Successfully merging a pull request may close this issue.

2 participants