moose icon indicating copy to clipboard operation
moose copied to clipboard

Detect OS and provide download link

Open ritz078 opened this issue 4 years ago • 11 comments

Right now we show all the download links for different OS. We should automatically detect the OS, the user is on and provide a single download link.

Screenshot 2021-01-31 at 3 08 56 PM

ritz078 avatar Jan 31 '21 09:01 ritz078

I would like to take up this issue.

agent515 avatar Feb 01 '21 07:02 agent515

I would like to take up this issue.

Sure. I have assigned this to you.

ritz078 avatar Feb 01 '21 08:02 ritz078

On Windows: image

On devices other than Linux or Mac: image

I have used platform.js to detect devices. I don't have Mac or Linux machines to show that now, but I'm sure they'll work like Windows.

But there is one problem. While rendering HTML, the GitHub button replaces the Apple button. Not able to find why even after trying a variety of things. Below is the code.

Apple logo is shown only when its code is directly placed in the return statement i.e. outside {}. But I need {} to apply conditional-javascript statements.

var platformOs = platform.os.toString();

var linuxDownloadButton = (<a href={linuxUrl} target="_blank">
                            <button className="download-button">
                              <Icon path={mdiLinux} size={1.2} />
                            </button>
                          </a>);
var macOSDownloadButton = (<a href={macUrl} target="_blank">
                            <button className="download-button">
                              <Icon path={mdiApple} size={1.2} />
                            </button>
                          </a>);
var windowsDownloadButton = (<button title="Coming soon" disabled className="download-button">
                              <Icon path={mdiMicrosoftWindows} size={1.2} />
                            </button>);
var githubRepoButton = (<a href="https://github.com/ritz078/moose" target="_blank">
                          <button className="download-button">
                            <Icon path={mdiGithub} size={1.2} />
                          </button>
                        </a>);

return (
   <div>
    ....
    
   <div>
      <img className="logo" src="/logo.svg" alt="" />

      <span>A torrent client to download, stream and cast torrents.</span>
      {
        platformOs.match(/Mac OS/i) ? (<div className="downloads">
          {macOSDownloadButton}
          {githubRepoButton}
        </div>) :
        (platformOs.match(/Win/i) ? (<div className="downloads">
          {windowsDownloadButton}
          {githubRepoButton}
        </div>) : 
        (platformOs.match(/Linux/i) ? (<div className="downloads">
          {linuxDownloadButton}
          {githubRepoButton}
        </div>) :
          (<div className="downloads">
            {macOSDownloadButton}
            {windowsDownloadButton}
            {linuxDownloadButton}
            {githubRepoButton}
          </div>))
        )
      }
    </div> 
 
    ....
   </div>
);

agent515 avatar Feb 02 '21 07:02 agent515

@ritz078 can you suggest what should I do?

agent515 avatar Feb 02 '21 17:02 agent515

Nested ternary operator is never a good idea. Why not simplify the code:

var platformOs = platform.os.toString();

var linuxDownloadButton = (
  <a href={linuxUrl} target="_blank">
    <button className="download-button">
      <Icon path={mdiLinux} size={1.2} />
    </button>
  </a>
);
var macOSDownloadButton = (
  <a href={macUrl} target="_blank">
    <button className="download-button">
      <Icon path={mdiApple} size={1.2} />
    </button>
  </a>
);
var windowsDownloadButton = (
  <button title="Coming soon" disabled className="download-button">
    <Icon path={mdiMicrosoftWindows} size={1.2} />
  </button>
);
var githubRepoButton = (
  <a href="https://github.com/ritz078/moose" target="_blank">
    <button className="download-button">
      <Icon path={mdiGithub} size={1.2} />
    </button>
  </a>
);

return (
  <div>
    ....
    <div>
      <img className="logo" src="/logo.svg" alt="" />

      <span>A torrent client to download, stream and cast torrents.</span>

      <div className="downloads">
        {platformOs.match(/Mac OS/i) && macOSDownloadButton}
        {platformOs.match(/Win/i) && macOSDownloadButton}
        {platformOs.match(/Linux/i) && linuxDownloadButton}
        {githubRepoButton}
      </div>
    </div>
    ....
  </div>
);

ritz078 avatar Feb 03 '21 06:02 ritz078

@ritz078 Ohh, that looks very neat. ✨ Should I make a PR then? and one more question. What do you use for syntax-highlighting in code-snippet?

agent515 avatar Feb 03 '21 06:02 agent515

Should I make a PR then?

You should have directly opened a PR. We can discuss code changes in the PR. Issue is just for discussing the problem. Not improving the code.

What do you use for syntax-highlighting in code-snippet?

https://docs.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks#syntax-highlighting

ritz078 avatar Feb 03 '21 06:02 ritz078

I'm afraid it is still showing GitHub button in place of Apple button.

image

agent515 avatar Feb 03 '21 06:02 agent515

Go ahead and open a PR. I will be able to see this in action there.

ritz078 avatar Feb 03 '21 06:02 ritz078

You should have directly opened a PR. We can discuss code changes in the PR. Issue is just for discussing the problem. Not improving the code.

Yeah will do now. Was a bit confused about that..

agent515 avatar Feb 03 '21 07:02 agent515

Oh. See the code again. There's a typo. 😛

<div className="downloads">
  {platformOs.match(/Mac OS/i) && macOSDownloadButton}
- {platformOs.match(/Win/i) && macOSDownloadButton}
+ {platformOs.match(/Win/i) && windowsDownloadButton} 
  {platformOs.match(/Linux/i) && linuxDownloadButton}
  {githubRepoButton}
</div>

Line no 3 should be windowsDownloadButton instead of macOSDownloadButton

ritz078 avatar Feb 03 '21 07:02 ritz078