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

CHANGE: added more information in the dialog to restart the editor. #1913

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

smnwttbr
Copy link
Collaborator

@smnwttbr smnwttbr commented Apr 24, 2024

Description

When the editor is restarted, pending import operations and windows are closed. The dialog has added information to inform the user of this. ISXB-608

Changes made

Modified dialog text.

image

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
    • FogBugz is marked as "Resolved" with next release version correctly set.
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

@smnwttbr smnwttbr requested a review from ekcoh April 24, 2024 03:43
@smnwttbr smnwttbr self-assigned this Apr 24, 2024
@unity-cla-assistant
Copy link

unity-cla-assistant commented Apr 24, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This small clarification looks good to me. I would however add @duckets as a reviewer on this PR since he is the technical writer on the team and can support in following language and communication consitency guidelines for this kind of user feedback. Include @duckets as reviewer on anything involving dialogs, UI text, public API docs or manual updates.

I will request changes though since the CHANGELOG.md file needs to be updated with a notice under the Changed header about this modification (even if non-functional it makes sense to include it and to get into process with this PR). Also changelog entry should link the public URL of the bug issue for convenience (see existing examples).

@ekcoh
Copy link
Collaborator

ekcoh commented Apr 24, 2024

In addition the PR is in draft status, you should convert it into a regular PR if there are no further changes planned on it. We typically use drafts only to get some feedback when work is pending / not finished.

@ekcoh
Copy link
Collaborator

ekcoh commented Apr 24, 2024

I also see you need to sign the license/CLA for CI to pass on this one. This is a one time thing you need to do to contribute to the repo.

@ekcoh ekcoh requested a review from duckets April 25, 2024 07:55
@smnwttbr smnwttbr marked this pull request as ready for review April 26, 2024 03:07
Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@duckets
Copy link
Collaborator

duckets commented Apr 30, 2024

Hi, two things:

  1. I assume we've exhausted all other options that don't require this workaround, such as whether it's possible to delay showing this dialog until all importing is complete?
  2. Doesn't restarting the editor automatically continue the import for anything that's not yet imported? When we say the user has to start (or restart) the import process after the editor restarts, where exactly is this button to do this?

@smnwttbr smnwttbr changed the title added more information in the dialog to restart the editor. CHANGE: added more information in the dialog to restart the editor. May 1, 2024
@duckets
Copy link
Collaborator

duckets commented Jun 13, 2024

I'm reluctant to approve this because although technically it explains to the user what to do, even with the improved text, I still think it's a bad user experience and, it's something that first-time Unity users have a high chance of encountering when importing their first demo project or starting their own first project using asset store assets. Therefore it has the potential to be one of the very first things any user experiences when starting their first project in the editor, which I think should be given significantly elevated priority as a user experience.

If it's at all possible, I think it needs looking at more closely and maybe going back to engineers for a solution that doesn't interrupt the package dependency import process.

Copy link
Collaborator

@duckets duckets left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in the discussion, approved because the text is now clearer, but overall I don't think it's acceptable to ask potentially brand-new users to restart the editor and then have to remember to restart the package import process. See full comment in PR conversation!

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

Successfully merging this pull request may close these issues.

4 participants