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

aem2doc parsing now done in da-collab #127

Closed
wants to merge 13 commits into from
Closed

aem2doc parsing now done in da-collab #127

wants to merge 13 commits into from

Conversation

bosschaert
Copy link
Contributor

@bosschaert bosschaert commented May 13, 2024

Description

da-collab now does its own aem2doc and vice versa parsing. This PR is to accommodate this on the client side.
It additionally contains a panel to show error messages from the server if there are any.

It also contains a bugfix to close the current websocket if the client navigates to a different document.

Related Issue

This is the client-side change for
adobe/da-collab#38

Screenshots

Of the new error panel.

Basic error message:
Screenshot 2024-05-14 at 10 55 48

Stack trace can be seen when expanding:
Screenshot 2024-05-14 at 12 38 03

How Has This Been Tested?

  • Locally with da-collab and da-admin
  • Unit tests

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

aem-code-sync bot commented May 13, 2024

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@bosschaert bosschaert changed the title aem2doc parsing now down in da-collab aem2doc parsing now done in da-collab May 13, 2024
@bosschaert
Copy link
Contributor Author

Note that the Playwright tests should pass again once adobe/da-collab#38 has been merged.

@auniverseaway
Copy link
Member

I don't think we should expose these error messages to the authors. No author will ever understand a stack trace and developers have the console for debugging. This does raise some interesting considerations on parsing and what we should show in the event of an error.

It's also a little scary we already think we will have errors. Do we know what those will be? 😬

@bosschaert
Copy link
Contributor Author

I don't think we should expose these error messages to the authors. No author will ever understand a stack trace and developers have the console for debugging. This does raise some interesting considerations on parsing and what we should show in the event of an error.

It's also a little scary we already think we will have errors. Do we know what those will be? 😬

Hi @auniverseaway, starting at the end. We're not expecting any specific errors. As a matter of fact, I have only experienced such an error once during development which was then quickly fixed by @karlpauls - after that I haven't seen any. On the other hand, as with any running process there is always the possibility of errors, albeit small.

So then the question becomes. What do we do if there is an error?

Some options:

  • Nothing - in this case the Author might be wondering why things aren't working.
  • Log to console - in this case the Author still doesn't see anything and is probably wondering why things don't work as the Author likely doesn't look at the console.
  • Show a message - the Author realises there is something wrong and contacts support.

So my thinking was that with a message the Author is alerted to a problem and guided to contact support. The stack trace in there could be used to send to support to help them fix the problem, but I agree that maybe this is too much for on the screen.

So while I don't anticipate errors popping up what do you think the strategy should be if we have one?

Errors are not logged to the console.
@bosschaert
Copy link
Contributor Author

This PR is now replaced by #163

@bosschaert bosschaert closed this May 24, 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

2 participants