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

Update client not found modal to make creating an issue more prominent #1426

Merged
merged 10 commits into from
Jun 25, 2024

Conversation

jerelmiller
Copy link
Member

@jerelmiller jerelmiller commented Jun 24, 2024

Updates the client not found modal to link to creating a GitHub issue more prominent.

  • Adds an additional sentence in the introduction paragraphs to suggest doing so
  • Adds a tertiary button in the modal to link to issue creating
Screenshot 2024-06-25 at 11 32 14 AM

@jerelmiller jerelmiller requested a review from a team as a code owner June 24, 2024 21:07
Copy link

relativeci bot commented Jun 24, 2024

#611 Bundle Size — 1.28MiB (+0.03%).

9568143(current) vs 54d39bd main#609(baseline)

Warning

Bundle contains 13 duplicate packages – View duplicate packages

Bundle metrics  Change 3 changes Regression 1 regression
                 Current
#611
     Baseline
#609
Regression  Initial JS 1.24MiB(+0.03%) 1.24MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 91.28% 97.11%
No change  Chunks 5 5
No change  Assets 12 12
Change  Modules 1028(+0.1%) 1027
No change  Duplicate Modules 49 49
No change  Duplicate Code 3.94% 3.94%
No change  Packages 162 162
No change  Duplicate Packages 10 10
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#611
     Baseline
#609
Regression  JS 1.24MiB (+0.03%) 1.24MiB
No change  IMG 35.85KiB 35.85KiB
No change  HTML 810B 810B
No change  Other 778B 778B

Bundle analysis reportBranch jerel/update-modal-messagingProject dashboard

@phryneas
Copy link
Member

Hmm, the "reached in error" now repeats and reads a bit contradictory for me. I don't have better wordings suggestions, though.
Still, could you noodle on this a little bit more? The last two paragraphs seem a bit inconsistent like this.

@jerelmiller
Copy link
Member Author

Ha thank you. You're totally right. I'll noodle on this a bit more. Thank you!

</Modal.Header>
<Modal.Body>
<p>
<Modal.Description>
An Apollo Client instance was not found, either because an Apollo
Client instance was never created, or because it could not be
discovered. This is most commonly fixed by setting the{" "}

Choose a reason for hiding this comment

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

Suggestion to

  • Use active voice to give ownership of the issue to the user
  • Refer to "this" as "this issue" to establish what "this" is. We can substitute "error" for "issue" if that is more accurate.
Suggested change
discovered. This is most commonly fixed by setting the{" "}
discovered. You can often solve this issue by setting {" "}

</Modal.Header>
<Modal.Body>
<p>
<Modal.Description>
An Apollo Client instance was not found, either because an Apollo
Client instance was never created, or because it could not be
discovered. This is most commonly fixed by setting the{" "}
<ConnectToDevToolsOptionLink /> to <code>true</code>.

Choose a reason for hiding this comment

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

Though it's in the ConnectToDevToolsOptionLink link once they open it, it can be helpful to give context about where this option lives.

Suggested change
<ConnectToDevToolsOptionLink /> to <code>true</code>.
<ConnectToDevToolsOptionLink /> in your <code>ApolloClientOptions</code> to <code>true</code>.

Comment on lines 42 to 44
If this was reached in error, please try one of the following
suggestions or click the &quot;Retry&quot; button to try looking for
the client again.
Copy link

@Meschreiber Meschreiber Jun 25, 2024

Choose a reason for hiding this comment

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

I think this is the flow we want to encourage? (Spacing might need to be fixed.)
Per our style guide, we use <strong> for text that appears in UIs.

Suggested change
If this was reached in error, please try one of the following
suggestions or click the &quot;Retry&quot; button to try looking for
the client again.
If you've already set <ConnectToDevToolsOptionLink /> to <code>true</code>, please try one of the following
suggestions or click the <strong>Retry</strong> button to try looking for
the client again.

>
create an issue
</GitHubIssueLink>{" "}
if you believe you have reached this in error.

Choose a reason for hiding this comment

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

Specify what "this" is. Use the same term, whether "issue," "error," or something else, as the first suggestion.

Suggested change
if you believe you have reached this in error.
if you believe you received this issue by mistake.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the help! Added a bit more verbosity here to really make it clear. f841d58

@jerelmiller jerelmiller merged commit 73e7755 into main Jun 25, 2024
9 checks passed
@jerelmiller jerelmiller deleted the jerel/update-modal-messaging branch June 25, 2024 17:42
@github-actions github-actions bot mentioned this pull request Jun 25, 2024
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.

None yet

3 participants