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

Fix error messages can't be copied #7115

Merged
merged 3 commits into from
Jan 21, 2022
Merged

Fix error messages can't be copied #7115

merged 3 commits into from
Jan 21, 2022

Conversation

WiXSL
Copy link
Contributor

@WiXSL WiXSL commented Jan 20, 2022

Fixes #6749

Before;
140480134-5697c857-b012-4517-b943-4d19c76b5da9

After:
error

@WiXSL WiXSL added the RFR Ready For Review label Jan 20, 2022
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

Not fan of the design.

Here is an alternative suggestion: the AccordionSummary has a fixed title (like "Error Stack") in your PR, and the AccordionDetails contains both the error (in bold) and the error stack.

It should make it copyable, no?

packages/ra-ui-materialui/src/layout/Error.tsx Outdated Show resolved Hide resolved
@WiXSL
Copy link
Contributor Author

WiXSL commented Jan 21, 2022

Not fan of the design.

Here is an alternative suggestion: the AccordionSummary has a fixed title (like "Error Stack") in your PR, and the AccordionDetails contains both the error (in bold) and the error stack.

It should make it copyable, no?

The thing with this idea is that you are showing an error and there is no actual error to display, until the user interacts with it.

@vercel vercel bot temporarily deployed to Preview – react-admin January 21, 2022 11:20 Inactive
@vercel vercel bot temporarily deployed to Preview – react-admin-storybook January 21, 2022 11:20 Inactive
@WiXSL
Copy link
Contributor Author

WiXSL commented Jan 21, 2022

The other option is to add css user-select : 'all' to the summary to solve the problem.

chrome-capture

@vercel vercel bot temporarily deployed to Preview – react-admin-storybook January 21, 2022 11:56 Inactive
@vercel vercel bot temporarily deployed to Preview – react-admin January 21, 2022 11:56 Inactive
Copy link
Member

@fzaninotto fzaninotto left a comment

Choose a reason for hiding this comment

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

I prefer that solution!

Note that we've adopted another solution in next (see 19d7a69), but I think your solution is better and should be backported.

@fzaninotto fzaninotto merged commit 7f0aac1 into master Jan 21, 2022
@fzaninotto fzaninotto deleted the fix-error-msg branch January 21, 2022 12:21
@WiXSL WiXSL added this to the v3.19.7 milestone Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to copy the error message in clipboard
2 participants