Open widget framework integration #1450
Conversation
| @@ -13,6 +13,8 @@ | |||
| {% if js_settings_json %} | |||
| <script type="text/javascript"> | |||
| var SETTINGS = {{ js_settings_json|safe }}; | |||
| var csrfToken = '{{ csrf_token }}'; | |||
There was a problem hiding this comment.
what's this for / is it necessary?
There was a problem hiding this comment.
That is for making requests to the django backend. It's a django generated token that the backend reads and verifies that it's not a cross-site request forgery. If your project has a different way of handling this, we can change it.
| @@ -292,6 +294,9 @@ class App extends React.Component<AppProps> { | |||
| path={`${match.url}privacy-statement`} | |||
| component={PrivacyPolicyPage} | |||
| /> | |||
| <Route path={match.url} render={() => { | |||
| return (<WidgetList widgetListId={SETTINGS.widgetListId} />) | |||
| }}/> | |||
There was a problem hiding this comment.
by putting this here, the widgets will appear on all pages, right?
There was a problem hiding this comment.
Some notes from a code review:
- Needs a rebase on master
- Travis is failing on both python and javascript tests. The python failures so far are just formatting, which you can fix by running
docker-compose run web black . - The version changes to the engine definitions need to get reverted.
- There are a lot of version bumps in
requirements.txt. I think you can fix this by reverting that fix and then runningpip-compile -P open-widget-frameworkto only affect that one depedency -
open-widget-frameworkshould be pinned inrequirements.into a specific version so our builds don't upgrade automatically - WidgetWrappers.js appears to reference some functions that don't exist or aren't imported
| } | ||
| } | ||
|
|
||
| class mitodlWidgetWrapper extends Component { |
There was a problem hiding this comment.
are the components written in this file used anywhere?
There was a problem hiding this comment.
Those are stubs but haven't been fully implemented yet. They also will probably be nicer if / when we implement redux in the package and then we won't need to pass through so many widget props.
|
@jklingen92 this is a really helpful start for us. @rhysyngsun is going to take this PR from here to do some of the more idiosyncratic stuff. |
|
Replaced by a newer PR #1534 |
Pre-Flight checklist
app.jsonWhat are the relevant tickets?
#1441
What's this PR do?
Initial integration of the open-widget-framework
How should this be manually tested?
No testing yet
Where should the reviewer start?
See if you can locally view, edit, add, and delete widgets
Any background context you want to provide?
There's still a lot to change and adjust. If the input is invalid (URLs especially) it won't add the widget
Screenshots (if appropriate)
(Optional)
What GIF best describes this PR or how it makes you feel?
(Optional)