Skip to content
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

URL path parts for section content #172

Open
iansk opened this issue Oct 1, 2023 · 8 comments · May be fixed by #189
Open

URL path parts for section content #172

iansk opened this issue Oct 1, 2023 · 8 comments · May be fixed by #189

Comments

@iansk
Copy link
Collaborator

iansk commented Oct 1, 2023

When clicking on a section in the left nav, content loads. However, the URL doesn't accurately (from a user perspective) reflect the "path" and exposes the underlying implementation details.

For example, open a browser tab, and go to this page: https://main--prisma-cloud-docs-website--hlxsites.hlx.live/prisma/prisma-cloud/en/enterprise-edition/content-collections/governance/governance

From the left nav, the path to the page appears to be: Enterprise Edition > Content Collections > Governance

However, the URL path is enterprise-edition/content-collections/governance/governance

The URL path should be enterprise-edition/content-collections/governance

@maxakuru
Copy link
Contributor

maxakuru commented Oct 2, 2023

there are a few ways to change this, but I think the most maintainable would be to use redirects from the prisma-cloud-docs repo and just change the frontend urls..

example: https://main--prisma-cloud-docs-website--hlxsites.hlx.page/en/enterprise-edition/content-collections/governance
redirects file: https://github.com/hlxsites/prisma-cloud-docs/blob/main/redirects.json

wdyt @iansk?

@iansk
Copy link
Collaborator Author

iansk commented Oct 3, 2023

@maxakuru What other options are there? We have tons of sections, and we're constantly refactoring content. It's going to be a headache on our side to continually maintain and update the redirects file. Is there an automatic way to handle this?

@maxakuru
Copy link
Contributor

maxakuru commented Oct 4, 2023

to automate it, the redirects.json could be built via a github action on merges to main, just like how sitemaps are generated & pushed

the other option is to build the required data into the worker & change the worker code to conditionally redirect requests to the "double end segment" version.. imo, that's a bit too much magic and would be difficult to debug. Having the list of redirects that are active there in code on the main branch would make debugging simple

@maxakuru
Copy link
Contributor

maxakuru commented Oct 5, 2023

I made a script that will generate redirects for topics that have the same path segment as their parent, you can see the output here: https://github.com/hlxsites/prisma-cloud-docs/blob/maxed/scripts/redirects.json

that combined with a tweak to how we create the sidenav links (#189) should fix this

@maxakuru maxakuru linked a pull request Oct 5, 2023 that will close this issue
@iansk
Copy link
Collaborator Author

iansk commented Oct 5, 2023

@maxakuru I'm concerned about how this will impact previews. As you know, I want previews to be rock solid, and fully reflect what you see in your preview will be what you see when you merge into main. Given that there is one redirects file, and there can be multiple branches with different/conflicting book.yml's, I think that this can get pretty messy pretty quickly.

Instead of the redirects, would refactoring book.yml be better?

kind: chapter
name: Connect
dir: connect
topics:
  - name: Connect
    file: connect.adoc
  - name: Connect Cloud Accounts
    dir: connect-cloud-accounts
    topics:
      - name: Connect Cloud Accounts
        file: connect-cloud-accounts.adoc
     - name: Onboard Your AWS Organization
        file: onboard-aws-org.adoc
      - name: Onboard Your AWS Account
        file: onboard-aws-account.adoc

Right now, we do a little magic under the hood in the left nav. For example, connect-cloud-accounts.adoc is hidden in the left nav, and rendered when you click the section drop-down "Connect Cloud Accounts". If we restructued the book.yml to better reflect what we want to do, could we fix the double connect-cloud-accounts/connect-cloud-accounts shown in the URL?

@maxakuru
Copy link
Contributor

maxakuru commented Oct 5, 2023

@iansk it doesn't matter what exists in book.yml, since that doesn't choose what document gets served. It's entirely based on the path to the document in the docs repo. If it's in a folder, that folder will be in the path for the docs repo. We can redirect from a simplified path to the actual path (either via redirects.json or via custom logic for specific cases) or we can move the doc.

when we're on the frontend and building the sidenav we can change what links are displayed easily, but those links only work if we know where the document actually exists in the docs repo. This is true for both the SPA navigation case (where we have the data inside book.yml and could, in theory, request a different doc) and for the initial load case, where we are blindly requesting /docs/<path>.plain.html on /<path> from the frontend

does that make sense?

@iansk
Copy link
Collaborator Author

iansk commented Oct 5, 2023

@maxakuru Yes, that makes sense. Let me consider a few options, and then reply back here first thing tomorrow morning so we can make a final decision.

@iansk
Copy link
Collaborator Author

iansk commented Oct 6, 2023

@maxakuru The approach we'd like to take is move the files so that the underlying path matches what we want to see in the URL. On our side, we'd need to update all our book.yml files and move all corresponding files. On your side, you'd need to update how Franklin consumes the book.yml, as well as front-end work for the sidenav. We also have an internal tool our writers use to validate book.yml - that too would need to be updated.

Undertaking this scale of work at this late stage introduces too much risk for the crisp delivery of our site on Oct 18.

This issue is still important to us, and we'd like to fix shortly after launch, so I'll keep this issue open. But for now, there's nothing for you to do.

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 a pull request may close this issue.

2 participants