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

Updates error view with new design #13197

Merged
merged 6 commits into from
Jun 2, 2022

Conversation

edmundito
Copy link
Contributor

@edmundito edmundito commented May 25, 2022

What

Updates error view with new design:

Onboarding example:

image

Full screen:
Screen Shot 2022-05-25 at 15 42 28

Figma:
https://www.figma.com/file/I5WCr153LfpLUuPTwsZZdD/ERROR_MESSAGES

Closes #12172

How

Applies the new design, migrates error view to use scss over styled components, and removes old error view navigation that have been replaced with a direct button instead of clicking on the Airbyte logo.

Recommended reading order

  1. ErrorOccurredView.tsx/.scss
  2. Rest of files to bottom

@edmundito edmundito requested a review from a team as a code owner May 25, 2022 19:46
@github-actions github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels May 25, 2022
Add the ability to set CTA button text and click handler
Migrate to SCSS module

Signed-off-by: Edmundo Ruiz Ghanem <edmundo@airbyte.io>
@edmundito edmundito force-pushed the edmundito/update-error-state-design branch from a82765c to 1b13814 Compare May 27, 2022 17:07
Copy link
Contributor

@teallarson teallarson left a comment

Choose a reason for hiding this comment

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

Works as expected when I tested it locally. Code lgtm.

Can you link to Figma in either the PR or the Issue so we have a record of it? I couldn't find the designs.

@vincentkoc
Copy link
Contributor

Looks great @edmundito

message={<FormattedMessage id="errorView.unknownError" />}
ctaButtonText={<FormattedMessage id="ui.goBack" />}
onCtaButtonClick={() => {
navigate("..");
Copy link
Collaborator

Choose a reason for hiding this comment

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

❓ Could you clarify why we're always wanting to go one level app, no matter where we are? This error boundary could be rendered in a lot of different pathes. Are we sure we're even having a valid route registered for all "one level" up navigations?

Copy link
Contributor Author

@edmundito edmundito May 31, 2022

Choose a reason for hiding this comment

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

Thanks for pointing this up. I rested the app and we do support one level-up navigation on all routes. The reasoning is that this will get users to the nearest area to where they currently are.

Co-authored-by: Tim Roes <tim@airbyte.io>
@edmundito edmundito requested a review from timroes May 31, 2022 13:26
Co-authored-by: Tim Roes <tim@airbyte.io>
);

.content {
margin-top: -17px; // Offset for octavia image padding
Copy link
Collaborator

Choose a reason for hiding this comment

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

To my understanding this is just caused by a larger transparent border around the image? Could we instead just crop the image properly, to not require to hack in SCSS some negative margin to move the div?

@edmundito edmundito requested a review from timroes May 31, 2022 15:34
@edmundito edmundito merged commit bf0a1c3 into master Jun 2, 2022
@edmundito edmundito deleted the edmundito/update-error-state-design branch June 2, 2022 18:07
chebalski added a commit to BluestarGenomics/airbyte that referenced this pull request Jul 25, 2022
* master: (142 commits)
  Highlight removed and added streams in Connection form (airbytehq#13392)
  🐛  Source Amplitude: Fixed JSON Validator `date-time` validation (airbytehq#13373)
  🐛 Source Mixpanel: publish v0.1.17 (airbytehq#13450)
  Fixed reverted PR: Fix cancel button when it doesn't provide feedback to the user + UX improvements (airbytehq#13388)
  🎉 Source Freshdesk: Added new streams (airbytehq#13332)
  Prepare YamlSeedConfigPersistence for dependency injection (airbytehq#13384)
  helm chart: Support nodeSelector, tolerations and affinity on the booloader pod (airbytehq#11467)
  airbyte-api: add jackson model annotations to remove null values from responses (airbytehq#13370)
  Change stage to `beta` (airbytehq#13422)
  🐛 Source Google Sheets: Retry on server errors (airbytehq#13446)
  Improve kube deploy process. (airbytehq#13397)
  Helm chart dependencies fix (airbytehq#13432)
  🐛 Source HubSpot: Transform `contact_lists` data to comply with schema (airbytehq#13218)
  airbytehq#11758: Source Google Ads to GA (airbytehq#13441)
  Add more pr actions to tag pull requests (airbytehq#13437)
  Source Google Ads: drop schema field that filters out the data from stream (airbytehq#13423)
  Updates error view with new design (airbytehq#13197)
  Source MSSQL: correct enum Standard method (airbytehq#13419)
  Update postgres doc about cdc publication (airbytehq#13433)
  run source acceptance tests against image built from branch (airbytehq#13401)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce more fine grained error boundaries
4 participants