-
Notifications
You must be signed in to change notification settings - Fork 23
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
ENDOC-570 mfe config #570
ENDOC-570 mfe config #570
Conversation
``` | ||
|
||
2. Replace the contents of `src/App.js` with the following. This component now displays the `name` property, turning the static component from the [React MFE tutorial](./react.md) into a more dynamic component. | ||
2. Replace the contents of `src/App.js` with the following. This component now receives the `config` property and displays the `name` parameter from it. This turns the static component from the [React MFE tutorial](./react.md) into a more dynamic component. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of "parameter from it" maybe "it contains" or "stored there"?
1. Generate a new React app | ||
```shell | ||
npx create-react-app my-widget-config --use-npm | ||
1. Add a new microfrontend to your bundle project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jyunmitch and i would like to punctuate every instruction followed by a code block with a colon, e.g. "Add a new microfrontend to your bundle project:"
); | ||
} | ||
} | ||
2. Generate a new React app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:
> Note: The generated ID of each file name (e.g. '073c9b0a') may change after every build. These folders may also contain LICENSE.txt or .map files, but they are not applicable to this tutorial. | ||
|
||
8. Go to `Components` → `MFE & Widgets` and edit your target widget | ||
1. Edit the `entando.json` and add these properties to the target microfrontend. This will enable the configuration MFE for the target MFE in the App Builder as well as the list of parameters to pick up from the config MFE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pick up -> pull ?
"This will enable the configuration MFE for the target MFE in the App Builder as well as the list of parameters to pick up from the config MFE."
are "for the target MFE" and "from the config MFE" (bolded) needed? is it understood that the config MFE services a target MFE and offers a selection of parameters?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lyd1aCla1r3 How about "...add these properties to the target MFE in order to connect the config MFE to the target MFE and specify the available params."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes :)
|
||
- `my-widget-config/static/css` | ||
- `my-widget-config/static/js` | ||
7. For test purposes, add a configuration file `public/mfe-config.json` with the following content. Note: a config MFE only receives a set of parameters from the App Builder, rather than the full config structure used for a regular MFE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"used for a regular MFE": used -> supplied ?; regular -> is there another word to describe the standard type of MFE?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
widget vs widget-config. I'll simplify it.
|
||
4. Replace the `body` of `public/index.html` with the following. This allows you to set the MFE `config` attribute and test locally with the same configuration structure provided by Entando. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and test locally with the same configuration structure provided by Entando. >> and locally test the same configuration structure provided by Entando. ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jyunmitch I don't think that has quite the same meaning. The idea is to test the MFE locally in a way that is similar to what Entando does. We're not actually testing the config structure, we're just setting up the MFE locally in the same way so we can test the MFE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the distinction based on the placement of the adverb is pretty subtle... what is the difference between "locally test" and "test locally"? the latter "sounds" right, but i'm struggling to put words to why
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lyd1aCla1r3 It's more about what is being tested - the MFE or the config structure. We're locally testing the MFE with the Entando structure, rather than locally testing the Entando config structure itself. If we changed Jinah's proposal to "locally test the config MFE with the same structure provided by Entando" then I'd be fine with the new language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And, I agree, it's subtle.
|
||
11. Publish the page and confirm the target MFE is configured and displays correctly | ||
4. Publish the page and confirm the target MFE is configured and displays correctly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing period
No description provided.