-
Notifications
You must be signed in to change notification settings - Fork 6
Core 901 add portal routing handler #2732
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
base: main
Are you sure you want to change the base?
Conversation
06becf5
to
cfcb24e
Compare
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.
what is here looks mostly good. i am a little concerned about the click handler changing the url, and wouldn't that produce an error with the urls that you're manually including the portal prefix on?
this doesn't seem to address the entire issue, especially the part about loading the flex page definition based on the portal name and using the layout config from the flex page (that is what makes it "portaly" when the flex page uses the landing page layout)
it would also be fine to merge this and then do the layout thing in a separate pr if you want
<Route path="/textbooks/:title" element={<RedirectToCanonicalDetailsPage />} /> | ||
<Route path="/subjects/*" element={<ImportedPage name="subjects" />} /> | ||
<Route path="/k12/*" element={<ImportedPage name="k12" />} /> | ||
<Route path="/blog/*" element={<ImportedPage name="blog" />} /> |
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.
i'm wondering if there are any of these, like k12, blog, general, that we explicitly don't want to work in the portal. i guess probably they're not hurting anything by being in there
a9716e6
to
8e0aeab
Compare
const {setLayoutParameters, layoutParameters} = useLayoutContext(); | ||
|
||
React.useEffect( |
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.
I was unable to see that this did anything.
8728053
to
fe82141
Compare
I have changed the link handling: I created a We might want to verify that the portal corresponds to an existing flex page before setting it? |
fe82141
to
6122918
Compare
CORE-901