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

Extract ui #248

Closed
wants to merge 7 commits into from
Closed

Extract ui #248

wants to merge 7 commits into from

Conversation

AdrianMrn
Copy link
Contributor

@AdrianMrn AdrianMrn commented Feb 12, 2020

Shares should be tested before merging this.

This PR extracts the UI into a separate package (hosted at @facadecompany/ignition-ui on npm), so the UI can be used by other error notifiers.

TODO: Remove the JS test from this repo's GitHub Actions, and move them to the ignition-ui repo.

@mpociot @freekmurze please check the following:
I changed this line:

return $shareReportAction->handle($request->get('report'), $request->get('tabs'), $request->get('lineSelection'));

and the ignition-ui package now also doesn't send the report as a string, but as an object. I don't know why this was implemented originally, but it doesn't seem to be causing any issues now. Do you think this could be a problem?

Read here for more information on why I did this: facade/ignition-js#13

@freekmurze
Copy link
Collaborator

We'll hold off on this for now.

@freekmurze freekmurze closed this Jul 14, 2020
@freekmurze freekmurze deleted the extract-ui branch May 17, 2021 10:36
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.

2 participants