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

Improve the UI/UX of the Export Template Manager dialog #48902

Merged

Conversation

YuriSizov
Copy link
Contributor

@YuriSizov YuriSizov commented May 20, 2021

With @Calinou's blessing supersedes #35568. This implements the mockup that I've described in that PR.

image
Original version

This is now a single window instead of two unnecessary dialogs which tended to get stuck in awkward states. The processes are streamlined, and the overall look of the UI is improved. The list of mirrors is refreshed automatically on popup if there is no template. Can also be manually updated. There is a quick download button now that refreshes the lists and picks the best (read here the first and only) mirror to start the download. The process can be canceled at any moment.

Current templates and the root templates folder can be opened with a click. Individual mirrors can be picked manually, or opened in a web browser, as before. Other templates can still be viewed and deleted.

2021-06-03_22-19-21.mp4
2021-06-03_22-20-34.mp4

Original version in action

I've decided not to abort the download when the window is closed. Instead, I think it'd be fine to keep the download going. The manager dialog respects that and will show the correct state upon a reopen. When closing with an active download there is a warning that informs that the process will continue and will be finished even if the window is never brought back up again. You do experience a short freeze, but it should be fine (and that warning does talk about it).

image

@YuriSizov YuriSizov added enhancement topic:editor usability cherrypick:3.x Considered for cherry-picking into a future 3.x release labels May 20, 2021
@YuriSizov YuriSizov added this to the 4.0 milestone May 20, 2021
@YuriSizov YuriSizov requested a review from a team May 20, 2021 20:25
@YuriSizov
Copy link
Contributor Author

Note: there is a bit of a discrepancy. When installing templates from a file, a progress dialog is displayed. However upon the end of the download this does not happen. This is because I was unable to show a progress dialog at the end of handling the completion of the related HTTP request. There is a check in the engine that triggers this error:

Do not use progress dialog (task) while flushing the message queue or using call_deferred().

Previous author here ended up conditioning the use of EditorProgress to disable it for the end of the HTTP request. I ended up doing the same, as I have no idea how to solve this conundrum. But it's not very noticeable to the user, as the process of unpacking is relatively quick.

@AaronRecord
Copy link
Contributor

Would close godotengine/godot-proposals#35?

@YuriSizov
Copy link
Contributor Author

YuriSizov commented May 20, 2021

Would close godotengine/godot-proposals#35?

Technically, yes. But I feel that to address this proposal some form of indication needs to be introduced to the main UI. Maybe a toast, if/when we get them.

@YuriSizov YuriSizov force-pushed the editor-improve-template-downloader branch from 72507d0 to 5f01ffc Compare May 31, 2021 14:56
@stijn-h
Copy link
Contributor

stijn-h commented May 31, 2021

I like the general idea of not having the download in a popup window. I do think that, compared to what we have now, your proposal seems less clear at a glance.

Some things that stood out to me:

  • The installed versions take up a lot of space, when they could even be hidden by default.
  • The download mirrors feels disconnected from the download button. This selection could actually benefit from a popup window, or it should be connected to the download button in another way.
  • The download button is not very visible. It coud have text, or be more prominent in another way. This is related to the other points: solving the first two issues might solve this too.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented May 31, 2021

@hinlopen

The installed versions take up a lot of space, when they could even be hidden by default.

They don't really take that space from any other control. The window can be made shorter, of course, but this is not a space that can be used on something else.

The download mirrors feels disconnected from the download button. This selection could actually benefit from a popup window, or it should be connected to the download button in another way.

The download mirrors have their own download button. The download button at the top is not supposed to be connected to them. If you want to download from a specific mirror, there is a button right next to it to do so. But keep in mind, in reality there is only one mirror, always.

The download button is not very visible. It coud have text, or be more prominent in another way.

We sure can change the icon on the top download button to the text. It would disrupt the state when the current template is already installed and there are other actions there too. Making all of them text may not be good, but we do have space, we can try that.

Edit:
Here're a couple of screenshots

Overall, I think it's good this way. Should we change everything to text then? Everything not in Trees maybe?

@groud
Copy link
Member

groud commented May 31, 2021

I agree with comments made by @hinlopen here. I would add few things:

  • The "install from file" button is placed next to the refresh button for download mirrors, which is weird. It's hard to understand that they are kind of unrelated to the list of mirrors.
  • It's not really clear which download mirror is currently selected. At least the selection icon should be made invisible if you already selected it.

But to be honest, I think the design needs some rework, it gives you way too much information you don't need, while most people will simply need to come and click the "Download" button, in which the current design is a lot better.
I think the download mirrors do not need to be listed here, it's something 99% users will never modify, so I think it would be better kept in the editor settings.

Also, I wonder if we should not simply add the "current" export template to the "installed" list. It would kind of make sense to treat all installed templates the same way (so that you can open their folder, see their path, or delete them). That would require the list to be improved a bit. Once downloaded, I guess the top line would simply prompt you with something like "Current export template already installed" or something like that.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented May 31, 2021

The "install from file" button is placed next to the refresh button for download mirrors, which is weird. It's hard to understand that they are kind of unrelated to the list of mirrors.

Yeah, I wasn't sure where to put it as before it replaced the "Ok" button of the dialog, which was weird. Putting it next to the installed list is also not obvious, IMO. So my logic was: this is a list of mirrors, alternatives to the top download button, and the "Install from File" button is also an alternative; so kind of makes sense...

Maybe it would make more sense if it was a text and not an icon:

image

It's not really clear which download mirror is currently selected. At least the selection icon should be made invisible if you already selected it.

You don't select mirrors, mirrors are never selected in this new UI. The download button at the top picks a mirror at its own discretion (and the tooltip explains it). If you want to download from a specific mirror, you press the download button next to that mirror:

image

I think the download mirrors do not need to be listed here, it's something 99% users will never modify, so I think it would be better kept in the editor settings.

We can hide mirrors completely, of course, but it is still useful to have a way to open them in a web browser if some network policies prevent the editor from downloading it.

But to be honest, I think the design needs some rework, it gives you way too much information you don't need, while most people will simply need to come and click the "Download" button, in which the current design is a lot better.

In current design this doesn't happen, though. You click "Download" and yet another weird popup shows up where you have to click on a link to start the download. And with this PR users will actually be able to just press "Download" and wait, without any other action required from them. That's what the top download button is for.

Also, I wonder if we should not simply add the "current" export template to the "installed" list.

This was the case before but was changed on purpose by some recent PR.

It would kind of make sense to treat all installed templates the same way (so that you can open their folder, see their path, or delete them).

I did consider that, but decided that most users won't really care for the templates that aren't useful to them in the current version. There is still a button that opens the root template folder. But we can add all that to the Tree, sure.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented May 31, 2021

I can see the point about the most important functions not being the most prominent (because they don't really take as much space as tree do, eh). Not sure how this can be addressed. Texts do help a little with the emphasis:

image

@YuriSizov YuriSizov force-pushed the editor-improve-template-downloader branch from 5f01ffc to 1f19604 Compare May 31, 2021 16:46
@stijn-h
Copy link
Contributor

stijn-h commented May 31, 2021

An easy solution is to put some more space between the installed versions and the download section, and if we keep the mirrors this way, maybe some way to emphasize the primary download. I still think adding a check button (off by default) that shows/hides the installed versions would help.

Also, if the user has installed the correct templates, instead of showing a path you could add a green check mark / "installed" message.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented May 31, 2021

An easy solution is to put some more space between the installed versions and the download section

While our UI language is inconsistent (otherwise this PR wouldn't have existed 😛), we don't really have empty space for padding in it. That separation needs to be achieved with something, not with the absence of something, I'm afraid.

I still think adding a check button (off by default) that shows/hides the installed versions would help.

A collapsible section would work there, but we don't have that in our UI either, I don't think. That's something that JetBrains does all the time, so I can see how it would work, but we don't have anything like that currently.

Also, if the user has installed the correct templates, instead of showing a path you could add a green check mark / "installed" message.

Yeah, but that's what we have currently, and it's useless. It tells nothing the user can't deduce from the existence of the path and the uninstall button. And path can be interacted with, used, copied. I see no merit in removing new information in favor of a void statement.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented May 31, 2021

Okay, how about a little change in the layout like this:

This also makes sure the "Install from File" button cannot be pressed while downloading, so it shouldn't cause any conflict.


I'm still thinking if we can somehow still provide mirrors yet make sure they are for advanced use only.

@Calinou
Copy link
Member

Calinou commented Jun 1, 2021

I'm still thinking if we can somehow still provide mirrors yet make sure they are for advanced use only.

It could be made an opt-in editor setting. For most people, picking a random mirror is fine (we only have 1 mirror currently anyway).

@YuriSizov YuriSizov force-pushed the editor-improve-template-downloader branch 3 times, most recently from 6836fba to 3e1cac5 Compare June 2, 2021 19:03
@YuriSizov
Copy link
Contributor Author

YuriSizov commented Jun 2, 2021

Okay, after hours moving elements around and discussing this on RC, I think this is the layout I am happy with. There are a lot of other opinions on what to change, but I can't compile it into an idea that satisfies everyone's taste, I'm afraid. I think this is good enough, and it solves more problems than it creates.

Note that the look is going to be a bit different after the recent theme changes in master, all labels are now bold by default:

Default state and downloading:

2021-06-02_21-39-54.mp4

Templates installed:

godot windows tools 64_2021-06-02_21-40-28

Context menu next to mirrors can be used to copy mirror URL or directly open it in a web browser:

godot windows tools 64_2021-06-02_21-47-49


It seems that some recent changes (#49026?) broke file downloading, so I cannot actually test it after the rebase I just did. I also cannot show the actual look with bold labels after the theme changes.

Edit: Yes, confirmed an issue with #49026 — to test this #49261 needs to be fixed.

@stijn-h
Copy link
Contributor

stijn-h commented Jun 3, 2021

I am not sure if Godot has styles for primary/secondary buttons, that could be used to distinguish the "Install from File" button. It really hurts having the most important button in the dialog have the same text and background color as a tree node, like come on.

The dialog is still not easy to read because of that. What if you:

  1. Add a icon (warning/error) to indicate that the template is missing.
  2. Next to the version set the mirror selection and download
  3. Put install from file button with file icon after a vertical bar.

You might have to make the dialog a bit wider or change "current version" to "active" or remove it entirely.

Tried to make a mock up, never doing that again 🙃:
image

@YuriSizov
Copy link
Contributor Author

YuriSizov commented Jun 3, 2021

I am not sure if Godot has styles for primary/secondary buttons

Not really.

Add a icon (warning/error) to indicate that the template is missing.

I don't think an icon is better than the explicit text in red. Especially now that all labels are bold by default (maybe subject to change, but currently it's like this).

godot windows tools 64_2021-06-03_14-53-40

You might have to make the dialog a bit wider

I've already received complaints that it's too wide as is. 🙃 Also your mock-up has some empty space for no apparent reason. If it didn't, the most important part of the UI — downloading and installing — would be tiny compared to the list of other installed version (which is only auxiliary information). Basically the same as the original layout in the OP.


Ultimately, like I've said above, I don't think I want to do any more changes. I've spend a day discussing this on RC and moving things around and everybody has an opinion that is slightly different than every other opinion. I just don't think we have a concept that is all-encompassing at this point. But we do have issues with the current version and I'd like this to be merged in its current form to address these issues.

We can always work on this flow more. Especially once we have more mirrors and this part becomes more important. And if this is merged we can at least see if we receive a lot of reports on support forums and community platforms from users misunderstanding the flow.

And please, do try this live if you didn't, don't trust the screenshots alone.

@YuriSizov YuriSizov force-pushed the editor-improve-template-downloader branch from 3e1cac5 to c7bb08f Compare June 3, 2021 19:21
@YuriSizov
Copy link
Contributor Author

Okay, downloading issues were fixed, this can now be properly tested with the latest master.

Here are additional previews of the current master theme (with bold labels):

2021-06-03_22-19-21.mp4
2021-06-03_22-20-34.mp4

And a static image of templates installed.

@stijn-h
Copy link
Contributor

stijn-h commented Jun 3, 2021

Ultimately, like I've said above, I don't think I want to do any more changes.

It's looking pretty good as it is, I was nitpicking a bit too much. The design indeed feels better in action than as a screenshot, and it is pretty easy to get used to it.

@akien-mga akien-mga merged commit 6aa3d8f into godotengine:master Jun 4, 2021
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

6 participants