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

Detect OS and provide download link #236

Open
ritz078 opened this issue Jan 31, 2021 · 11 comments · May be fixed by #254
Open

Detect OS and provide download link #236

ritz078 opened this issue Jan 31, 2021 · 11 comments · May be fixed by #254
Assignees
Labels

Comments

@ritz078
Copy link
Owner

ritz078 commented Jan 31, 2021

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

@agent515
Copy link

agent515 commented Feb 1, 2021

I would like to take up this issue.

@ritz078
Copy link
Owner Author

ritz078 commented Feb 1, 2021

I would like to take up this issue.

Sure. I have assigned this to you.

@agent515
Copy link

agent515 commented Feb 2, 2021

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
Copy link

agent515 commented Feb 2, 2021

@ritz078 can you suggest what should I do?

@ritz078
Copy link
Owner Author

ritz078 commented Feb 3, 2021

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>
);

@agent515
Copy link

agent515 commented Feb 3, 2021

@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?

@ritz078
Copy link
Owner Author

ritz078 commented Feb 3, 2021

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

@agent515
Copy link

agent515 commented Feb 3, 2021

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

image

@ritz078
Copy link
Owner Author

ritz078 commented Feb 3, 2021

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

@agent515
Copy link

agent515 commented Feb 3, 2021

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..

@ritz078
Copy link
Owner Author

ritz078 commented Feb 3, 2021

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

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

Successfully merging a pull request may close this issue.

2 participants