Skip to content

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

RoyEJohnson
Copy link
Contributor

@RoyEJohnson RoyEJohnson commented Apr 25, 2025

@RoyEJohnson RoyEJohnson force-pushed the core-901-add-portal-routing-handler branch 4 times, most recently from 06becf5 to cfcb24e Compare April 25, 2025 22:56
@RoyEJohnson RoyEJohnson requested a review from TomWoodward April 25, 2025 22:56
Copy link
Member

@TomWoodward TomWoodward left a 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" />} />
Copy link
Member

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

@RoyEJohnson RoyEJohnson force-pushed the core-901-add-portal-routing-handler branch from a9716e6 to 8e0aeab Compare April 29, 2025 15:45
const {setLayoutParameters, layoutParameters} = useLayoutContext();

React.useEffect(
Copy link
Contributor Author

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.

@RoyEJohnson RoyEJohnson force-pushed the core-901-add-portal-routing-handler branch from 8728053 to fe82141 Compare April 29, 2025 21:04
@RoyEJohnson
Copy link
Contributor Author

I have changed the link handling: I created a rewriteLinks utility in the PortalContext that rewrites the href of any links in a given element to add the portal if portal is set.
The footer element calls it, and so does the raw-html component, which is used in the adoption and interest form headers.I directly rewrote links in a few other pages.
I updated DefaultLayout to use a landing page layout when the portal is set.
It assumes a flex page with a landing page layout is a portal, but also any path that has a portal and something that loads successfully will be treated as a portal.
So these are all portal pages:
http://localhost:3000/b2s-fall-24
http://localhost:3000/b2s-fall-24/details/books/biology-2e
http://localhost:3000/b2s-fall-24/careers
http://localhost:3000/not-a-page/careers

We might want to verify that the portal corresponds to an existing flex page before setting it?

@RoyEJohnson RoyEJohnson requested a review from TomWoodward April 29, 2025 21:11
@RoyEJohnson RoyEJohnson force-pushed the core-901-add-portal-routing-handler branch from fe82141 to 6122918 Compare May 7, 2025 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants