[App Extensibility] Detect conflicting homepage url (@W-17599554@) #2215
Conversation
If there is, remove its route.
vmarta
added
the
skip changelog
The <ul> tag cannot be nested inside a <p>.
We needed to isolate the css styles to prevent leaking.
- Removed the gradient background - Content is now mostly static, but dynamically list the installed app extensions
| </a> | ||
| </p> | ||
|
|
||
| <p>You have installed the following application extensions:</p> |
There was a problem hiding this comment.
The typescript-minimal homepage now has a more minimal content.
| 100% { opacity: 1 } | ||
| } | ||
| .fade-in { | ||
| .content { |
There was a problem hiding this comment.
Fixed and constrained the css selectors to avoid the styles leaking to other unexpected elements.
| @@ -48,6 +49,166 @@ | |||
| "mobify": { | |||
| "app": { | |||
| "extensions": [ | |||
| ["@salesforce/extension-chakra-storefront", { | |||
There was a problem hiding this comment.
I'll revert this change before the PR is merged.
| .button { | ||
| display: inline-block; | ||
| padding: 10px 20px; | ||
| font-size: 16px; | ||
| font-weight: bold; | ||
| color: white; | ||
| background-color: #0176D3; | ||
| text-align: center; | ||
| text-decoration: none; | ||
| border-radius: 5px; | ||
| } |
There was a problem hiding this comment.
I like that it looks like a chakra button without being one 👍
| </p> | ||
| )) | ||
| ) : ( | ||
| <li>None</li> |
There was a problem hiding this comment.
This "none" is very computer sounding. Maybe we can make that look a little better. Like:
If you have extensions installed:
You have 2 extensions installed.
- Chakra Storefront
- Chakra Store Locator
If you don't have extensions installed:
You currently do not have any extensions installed.
There was a problem hiding this comment.
This by no means is what we'll have in the end. But it's a little cleaner. It would be nice to have friendly names like my examples above, but using the package name is ok for now.
It was complaining about the unnecessary type assertion.
| ` | ||
|
|
||
| const Home = ({value}: Props) => { | ||
| const [counter, setCounter] = useState(0) | ||
| const Home = () => { |
There was a problem hiding this comment.
Is there any reason why we don't want to called this "Start" or "GettingStarted"? I personally like the later, but I would that the path is in alignment with the page name.. so /_pwa-kit/getting-started and GettingStarted or /_pwa-kit/start and Start
There was a problem hiding this comment.
Agreed with Ben on this
| @@ -27,6 +27,9 @@ const options = { | |||
| // except by Safari. | |||
| protocol: 'http', | |||
|
|
|||
| // The path that the local dev server would open initially | |||
| startPath: '/__pwa-kit/start', | |||
There was a problem hiding this comment.
Nit: it seems like there is a small chance the Home page will result in a 404 if the value here is mismatched with the one in the path in routes. I don't think creating a constant would make much sense since it is likely to change when devepers starts to alternate the routes during developerment. Maybe leaving a comment in the routes to give more context for this value?
This PR handles the scenario of an app extension adding a homepage at the root
/. For example: the typescript-minimal template has its own homepage, but if you use the chakra-storefront extension, it'll also try to add its own homepage at the same url. Thus, there are 2 routes that target the same/path.With this PR, there will no longer be a conflict in the routes:
When the dev server runs, it'll open at
/__pwa-kit/startpath by default. The base template's homepage will thus be rendered.while the storefront extension's homepage is still at the
/route.Fix tests and maybe add new ones
How to test-drive the PR
node packages/pwa-kit-create-app/scripts/create-mobify-app-dev.js --outputDir /tmp/foonpm startWith storefront extension installed:
Without any extensions installed:
