scratchfoundation / scratch-gui

Graphical User Interface for creating and running Scratch 3.0 projects.
https://scratchfoundation.github.io/scratch-gui/develop/
BSD 3-Clause "New" or "Revised" License
4.42k stars 3.51k forks source link

Can't middle/ctrl/cmd-click account dropdown to open links in a separate tab #4241

Open towerofnix opened 5 years ago

towerofnix commented 5 years ago

Expected Behavior

Holding control or command while clicking, or middle-clicking, should open links in the account dropdown in a separate tab.

Actual Behavior

All those cases load the link in the tab used by the editor.

Steps to Reproduce

Open this dropdown menu, then click it in one of the ways described above.

The account dropdown in the top right of the online editor, including options "Profile", "My Stuff", "Account settings", "Sign out"

Note that the "my stuff" icon is an actual <a> link, and so it has all the usual expected behavior of <a> links (e.g. also having cursor: pointer by default, something the account dropdown is glaringly missing). As such, options in the account dropdown should all be made to be <a>s.

danfarrow commented 5 years ago

I think this is fixed / no longer an issue.

The markup for the account dropdown now uses <a> tags and has this structure:

<ul class="dropdown production open">
  <li>
    <a href="/users/my-username/">
      <span>Profile</span>
    </a>
  </li>
  <li>
    <a href="/mystuff/">
      <span>My Stuff</span>
    </a>
  </li>
  <!-- etc -->
</ul>
towerofnix commented 5 years ago

@danfarrow That's only the case on scratch-www, not for the menu in scratch-gui, where the structure is like this:

<ul class="..">
  <li class="..">
    <span>Profile</span>
  </li>
  <li class="..">
    <span>My stuff</span>
  </li>
  <!-- etc -->
</ul>
danfarrow commented 5 years ago

@towerofnix Oh whoops I was not aware of scratch-www! I had been following this project thinking it was the scratch 3.0 web gui. Forgive my ignorance but how does scratch-gui differ/where is it deployed?

towerofnix commented 5 years ago

@danfarrow No worries! The LLK repositories are a bit of a labyrinth to navigate. :)

The scratch-www repository is used as the implementation for all the React-based pages on the Scratch website; for example, the homepage, the privacy policy page, and the messages page. (But not pages that are yet to be "migrated" to the more modern React, such as the My Stuff page; those are in a private repository called scratchr2.)

The scratch-gui repository is the main place where Scratch 3.0's implementation resides. It's important to contrast between Scratch 3.0 and the migrated Scratch website; the former refers to the programming language and its editor/player, while the latter refers to the actual website deployed onto https://scratch.mit.edu.

In order to display Scratch 3.0 on the Scratch website, scratch-www uses scratch-gui as an NPM dependency. Then it embeds scratch-gui into the view for the project page, just like any other React element.

scratch-gui has a "playground" mode, very similar to the standalone mode used in Scratch Desktop, which is automatically deployed to https://llk.github.io/scratch-gui/develop/. This is the same environment you get when you run Scratch 3.0 locally, i.e. via npm start from the terminal and navigating to http://localhost:8601/.

Also, scratch-gui has dependencies of its own; particularly, it depends on scratch-vm, scratch-blocks, scratch-render, scratch-paint, and a host of other smaller repositories. These each provide core parts of the Scratch 3.0 editor and player, and are separate components, so they are put into their own repositories for code and issue tracker organization. (All of these are public / open source.) scratch-gui is the part which provides the rest of the actual editor, and helps to connect all the other core parts I mentioned above.

Hopefully this mini-guide helps explain the structure of LLK's repositories! I'm happy to explain further if you're wondering about anything.

danfarrow commented 5 years ago

@towerofnix Ah I see now, I had only looked at the Scratch homepage & /mystuff/

scratch-gui is pretty much what I thought it was but I hadn't considered that it would have its own markup for the dropdown menu.

It makes sense now though. Thanks for the mini-guide!

I'm hoping to contribute if I can get my head around React a bit more. I tried back in June and didn't get very far but I'll have another go at it

towerofnix commented 5 years ago

@danfarrow No problem! Yeah, scratch-gui's menu is a little different. I honestly haven't looked at its code much (although I've worked with a lot of other parts of the editor).

I think Scratch 3.0 is actually a pretty good introduction to working with React. I know I was basically React-clueless when I first got working with it, but, while I'm not certain I could invent my own React-based site altogether, I feel a lot more confident with working with it (and Redux) now that I've been contributing to Scratch 3.0 with React code for around a year.

benjiwheeler commented 5 years ago

About this issue in general -- one of the tricky things is that some menu items (profile, my stuff, etc.) are links that will take you to another page, while others (undo, turbo, save, download, upload, etc.) are triggers for actions within the page. Even the menu items that will navigate the user away from the page often first do things like trigger an auto-save. It's not totally clear to me what the middle click (or command-click on Mac) behavior should be, or how to implement it.

towerofnix commented 5 years ago

It's not totally clear to me what the middle click (or command-click on Mac) behavior should be, or how to implement it.

My two-cents:

  1. For options that don't navigate away from the page (undo, turbo, etc), middle-click should probably just behave like a normal click; this is the current behavior.
  2. For things that do navigate away, middle/cmd-click should certainly open the target page in a new tab; after all this is what users expect links to do. There is no need to worry about autosaving or other events, because middle-clicks don't close the page - they open up the link in a new tab/window.

To implement that latter one, we need to make those links into <a>s. We do still need to keep the behavior of auto-saving before navigating away from the page, though; in order to do that, we should keep the existing click event listeners, but only have the listener do anything (and simultaneously event.preventDefault()) when the click is primary (left button) and no modifiers are pressed (we'll leave it up to the browser then, which will most likely open the link in a new tab).

If we ever don't call preventDefault() before clicking the link navigates us away, that's okay; we have window.onbeforeunload as a fail-safe, after all. That would make sure the user never closes the page (by clicking a <a> link) when there are unsaved changes.