Skip to content

Remove unused /embedded/projects/:identifier route #1013

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

Merged
merged 1 commit into from
May 17, 2024

Conversation

floehopper
Copy link
Contributor

@floehopper floehopper commented May 17, 2024

As far as we can tell, this route is no longer used anywhere.

This was originally added in Mar 2022 in #72 for the "ESA demo". I suspect we now achieve a similar effect by using the web component with its embedded attribute set to true, rather than using an iframe with its src set to this route.

I found it confusing to see both the /:locale/embed/viewer/:identifier and this route in the AppRoutes component. Removing this one makes the code less confusing.

I've also removed the now redundant embedded property on the ProjectComponentLoader component and the similarly redundant call to useEmbeddedMode which is a no-op when embedded is false.

As far as we can tell, this route is no longer used anywhere [1].

This was originally added in Mar 2022 in #72 for the "ESA demo". I
suspect we now achieve a similar effect by using the web component
rather than using this route in combination with an `iframe`.

I found it confusing to see both the `/:locale/embed/viewer/:identifier`
and this route in the `AppRoutes` component. Removing this one makes the
code less confusing.

I've also removed the now redundant `embedded` property on the
`ProjectComponentLoader` component and the similarly redundant call to
`useEmbeddedMode` which is a no-op when `embedded` is `false`.

[1]: https://github.com/search?q=org%3Araspberrypilearning+embedded%2Fprojects&type=code
Copy link

Copy link

sra405
sra405 previously approved these changes May 17, 2024
Copy link
Contributor

@sra405 sra405 left a comment

Choose a reason for hiding this comment

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

👍 nice, definitely simplifies the loader!

@sra405 sra405 dismissed their stale review May 17, 2024 15:05

Lois is on it

Copy link
Contributor

@loiswells97 loiswells97 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@floehopper floehopper merged commit 0dbfc2c into main May 17, 2024
@floehopper floehopper deleted the remove-unused-embedded-projects-route branch May 17, 2024 15:31
floehopper added a commit that referenced this pull request Jun 3, 2024
Strictly speaking, I think the fact we’ve removed a route [1] means we
should probably bump the major version, but given (a) we’re still on
v0.x.x which is less strict about semantic versioning; and (b) we’re
confident nothing is using that route any more, we've decided [2] to
just to bump the minor version, i.e. go for release v0.24.0.

[1]: #1013
[2]: https://raspberrypifoundation.slack.com/archives/C02JBAA2NFP/p1717429841916009
@floehopper floehopper mentioned this pull request Jun 3, 2024
floehopper added a commit that referenced this pull request Jun 4, 2024
### Added

- Add `project_name_editable` attribute to web component (#1009)
- Fires custom event when the theme changes (#1015)
- Add `output_only` attribute to web component (#1019 & originally #782)
- Add `assets_identifier` attribute to web component (#1019 & originally
#901)
- Enhance `code` attribute on web component to override project main
component content (#1019 & originally #901)
- Add `runCode`, `stopCode` & `rerunCode` methods to web component
(#1019 & originally #899)
- Send error details in "editor-runCompleted" event (#1019 & originally
#915)
- Return error details to web component (#1019 & originally #915)
- Add `output_panels` attribute to web component (#1019 & originally
#909)

### Changed

- Remove unused `/embedded/projects/:identifier` route (#1013)

### Fixed

- Remove unused `REACT_APP_LOGIN_ENABLED` env var (#1006)
- Fix infinite remix loop when `BYPASS_AUTH` set in `editor-api` (#1007)
- Fixes for docker-compose.yml (#1008)
- Fix deprecation warnings in GitHub Actions (#1011)
- Removed unused `isEmbedded` param from `useProject` call in
`EmbeddedViewer` (#1016)
- Improvements to Cypress specs in CI (#1017)
- Fix warnings and verbose output when starting Webpack Dev Server
(#1018)
- Add e2e spec for project remix behaviour in web component (#1020)
- Fix initial value of `user` in `WebComponentLoader` (#1021)
- Make `authKey` in e2e web component spec more realistic (#1022)
- Remove unused `ComponentStore` (#1023)
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.

3 participants