Code and assets identifiers#901
Merged
loiswells97 merged 10 commits intooutput-only-wcfrom Jan 25, 2024
Merged
Conversation
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
loiswells97
temporarily deployed
to
previews/add-assets-identifier
GitHub Actions
Inactive
|
conorriches
reviewed
Jan 25, 2024
| export const loadAssets = async (assetsIdentifier, locale, accessToken) => { | ||
| const queryString = locale ? `?locale=${locale}` : ""; | ||
| return await get( | ||
| `${host}/api/projects/${assetsIdentifier}/images${queryString}`, |
Contributor
There was a problem hiding this comment.
I'm unfamiliar with the admin side, will the query string ever change the assets returned? e.g. can we have a Spanish locale image vs an English locale image?
Contributor
Author
There was a problem hiding this comment.
I guess potentially, maybe if there's text in there or something, or they want to provide a more culturally relevant image? The locale query string makes it load up a different project anyway, so it is possible to do this.
conorriches
reviewed
Jan 25, 2024
Contributor
conorriches
left a comment
conorriches
left a comment
There was a problem hiding this comment.
Just a thought or two, but looking good so far
Co-authored-by: Conor <905676+conorriches@users.noreply.github.com>
floehopper
added a commit
that referenced
this pull request
May 17, 2024
This seems extraneous to the other changes in #901.
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
So that it overrides the content of the main component in the loaded project. Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
Based on a curated set of changes from #901. TODO: Explain what web component `embedded` attribute does
floehopper
pushed a commit
that referenced
this pull request
May 20, 2024
Based on a curated set of changes from #901. TODO: Explain what web component `embedded` attribute does
floehopper
pushed a commit
that referenced
this pull request
May 22, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 22, 2024
So that it overrides the content of the main component in the loaded project. Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 22, 2024
TODO: Explain what web component `embedded` attribute does Based on a curated set of changes from #901. Note that in the original PR, an unnecessary call to `dispatch(setEmbedded(embedded))` was added in `WebComponentLoader`. This was unnecessary, because there was already a call to the `useEmbeddedMode` hook which does the same thing, so I have removed it from this commit.
floehopper
pushed a commit
that referenced
this pull request
May 22, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 22, 2024
So that it overrides the content of the main component in the loaded project. Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 22, 2024
TODO: Explain what web component `embedded` attribute does Based on a curated set of changes from #901. Note that in the original PR, an unnecessary call to `dispatch(setEmbedded(embedded))` was added in `WebComponentLoader`. This was unnecessary, because there was already a call to the `useEmbeddedMode` hook which does the same thing, so I have removed it from this commit.
floehopper
pushed a commit
that referenced
this pull request
May 28, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 28, 2024
So that it overrides the content of the main component in the loaded project. Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 28, 2024
TODO: Explain what web component `embedded` attribute does Based on a curated set of changes from #901. Note that in the original PR, an unnecessary call to `dispatch(setEmbedded(embedded))` was added in `WebComponentLoader`. This was unnecessary, because there was already a call to the `useEmbeddedMode` hook which does the same thing, so I have removed it from this commit.
floehopper
pushed a commit
that referenced
this pull request
May 29, 2024
Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 29, 2024
So that it overrides the content of the main component in the loaded project. Based on a curated set of changes from #901.
floehopper
pushed a commit
that referenced
this pull request
May 30, 2024
Based on a curated set of changes from #901. The new `assets_identifier` attribute is intended to be used in conjunction with the `code` attribute so that the supplied source code can reference images included in the project. The motivation for this change is for the Block-to-Text app [1]. I've also added unit tests after the fact - hopefully they make sense! [1]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/
floehopper
pushed a commit
that referenced
this pull request
May 30, 2024
So that it overrides the content of the main component in the loaded project. This means that for example, you can set the `assets_identifier` to bring in some images from a real project, but override the main component source code in `main.py` or `index.html`. The motivation for this change is to support the Block-to-Text app [1]. Based on a curated set of changes from #901. I've also added a bunch of unit tests which hopefully explain a bit about the detail of the behaviour. In doing that I noticed what I think is a small bug when the real project has other non-main components (i.e. files), but I plan to fix that in a subsequent commit. [1]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/
floehopper
pushed a commit
that referenced
this pull request
May 30, 2024
Based on a curated set of changes from #901. The new `assets_identifier` attribute is intended to be used in conjunction with the `code` attribute so that the supplied source code can reference images included in the project. The motivation for this change is for the Block-to-Text app [1]. I've also added unit tests after the fact - hopefully they make sense! [1]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/
floehopper
pushed a commit
that referenced
this pull request
May 30, 2024
So that it overrides the content of the main component in the loaded project. This means that for example, you can set the `assets_identifier` to bring in some images from a real project, but override the main component source code in `main.py` or `index.html`. The motivation for this change is to support the Block-to-Text app [1]. Based on a curated set of changes from #901. I've also added a bunch of unit tests which hopefully explain a bit about the detail of the behaviour. In doing that I noticed what I think is a small bug when the real project has other non-main components (i.e. files), but I plan to fix that in a subsequent commit. [1]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/
floehopper
pushed a commit
that referenced
this pull request
May 31, 2024
Based on a curated set of changes from #901. The new `assets_identifier` attribute is intended to be used in conjunction with the `code` attribute so that the supplied source code can reference images included in the project. Setting the attribute results in a call to the `Api::Projects::ImagesController#show` action [1] in `editor-api` which renders the `app/views/api/projects/images.json.jbuilder` template [2] which makes just the images for the project available; not the source files. The motivation for this change is for the Block-to-Text app [3] which uses the new attribute in its exercise to load the images for the "bridge-prototype" project [4,5]. I've also added unit tests to test the new behaviour. I plan to add an e2e spec in a subsequent commit which will test all the new attributes on the web component. [1]: https://github.com/RaspberryPiFoundation/editor-api/blob/96f435fbd9c2b1fb35821dd98e30a97056c6501a/app/controllers/api/projects/images_controller.rb#L8-L12 [2]: https://github.com/RaspberryPiFoundation/editor-api/blob/96f435fbd9c2b1fb35821dd98e30a97056c6501a/app/views/api/projects/images.json.jbuilder [3]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/ [4]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/blob/d16a8ecc1d3da202363fb16db31f4bf93f46aa04/src/components/Editor.js#L15 [5]: https://github.com/raspberrypilearning/bridge-prototype
floehopper
pushed a commit
that referenced
this pull request
May 31, 2024
So that it overrides the content of the main component in the loaded project. This means that for example, you can set the `assets_identifier` to bring in some images from a real project, but override the main component source code in `main.py` or `index.html`. The motivation for this change is to support the Block-to-Text app [1]. Its exercise uses the `code` attribute to load the relevant code for each step and modify it as the user makes changes [2]. This is based on a curated set of changes from #901. I've also added a bunch of unit tests which hopefully explain a bit about the detail of the behaviour. In doing that I noticed what I think is a small bug when the real project has other non-default components (i.e. files other than `main.py` or `index.html`), but I plan to fix that in a subsequent commit. I plan to add an e2e spec in a subsequent commit which will test all the new attributes on the web component. [1]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/ [2]: https://github.com/RaspberryPiFoundation/block-to-text-alpha/blob/d16a8ecc1d3da202363fb16db31f4bf93f46aa04/src/components/Editor.js#L17
floehopper
added a commit
that referenced
this pull request
May 31, 2024
I think this was the original intention in the changes I've included from #901, but there was a bug in the boolean logic. The `otherComponents` should include all components other than `main.py` & `index.html`. Previously it was typically not include any other components, because the `component.extension !== defaultExtension` expression was `false` for all of them, since in a Python project all the files would have the `.py` (i.e. default) extension.
Merged
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)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #896