[docs-infra] Improve product mapping #37729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LukasTy
reviewed
Jun 26, 2023
Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Comment on lines
-6
to
-20
| { | ||
| pathname: '/getting-started', | ||
| icon: standardNavIcons.DescriptionIcon, | ||
| children: [ | ||
| { pathname: '/getting-started/installation' }, | ||
| { pathname: '/getting-started/usage' }, | ||
| { pathname: '/getting-started/example-projects' }, | ||
| { pathname: '/getting-started/templates' }, | ||
| { pathname: '/getting-started/learn', title: 'Learning resources' }, | ||
| { pathname: '/getting-started/faq', title: 'FAQs' }, | ||
| { pathname: '/getting-started/supported-components' }, | ||
| { pathname: '/getting-started/supported-platforms' }, | ||
| { pathname: '/getting-started/support' }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Legacy, dead code
Comment on lines
+42
to
50
| if (firstFolder === 'x') { | ||
| productCategoryId = 'x'; | ||
| productId = `x-${asPathWithoutLang.replace('/x/react-', '').replace(/\/.*/, '')}`; | ||
| } | ||
|
|
||
| if (asPathWithoutLang === '/versions/') { | ||
| productId = null; | ||
| // No match, give up on it. | ||
| if (productId === 'x-') { | ||
| productId = 'null'; | ||
| } | ||
| } |
There was a problem hiding this comment.
With the changes in this PR, we could in theory move this logic to http://github.com/mui/mui-x. To complement #9451. But one step at a time. This is made possible by no longer relying on getProductInfoFromUrl to label the Algolia search results.
| @@ -85,7 +85,7 @@ const products = [ | |||
| ]; | |||
|
|
|||
| export default function MuiProductSelector() { | |||
| const { productId } = React.useContext(PageContext); | |||
| const pageContext = React.useContext(PageContext); | |||
There was a problem hiding this comment.
So much easier for large scale code refactoring.
Comment on lines
+343
to
+346
| // Extra | ||
| 'productId', | ||
| 'productCategoryId', | ||
| ], |
There was a problem hiding this comment.
In Algolia Crawler, we get these values like this:
{
productId: $('meta[name="mui:productId"]').attr("content"),
productCategoryId: $('meta[name="mui:productCategoryId"]').attr("content"),
}
|
May want to incorporate the edge cases in https://github.com/mui/material-ui/pull/37721/files and close that one |
|
@Janpot Ah yes, great. I have updated this PR. |
Janpot
approved these changes
Jun 28, 2023
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a continuation of #37593, to be able to segment the traffic on GA4 between products, it fixes bugs at the same time.
product->productIdto make it easier to navigate the codebase, better be clear as well.productIdstored in Algolia for the matching, rather than trying to re-compute the value from the pathname, this is wrong.getProductInfoFromUrl()to be simpler a step toward the futureThe next steps would be:
getProductInfoFromUrl()is hybrid, it feels like a leaky abstractionproductIdoverride the logic ofgetProductInfoFromUrl(). There are cases when the URL structure is not straightforward.useDemoData()has its own product id logic, this duplication is wrong. it should rely on thePageContext, no reasons to be different.