Search box is not aligned to the index nav width on the release notes page

commercetools / commercetools-docs-kit

Tools and components for developing Documentation websites 🛠

https://docs-kit.commercetools.vercel.app

MIT License

18 stars 5 forks source link

Search box is not aligned to the index nav width on the release notes page #2085

Closed ColinRosati closed 1 month ago

ColinRosati commented 2 months ago

Description of changes

Align layout header and layout page content for release notes
util for shared layout for header and page content
refactor grid layouts
- Shared const IDs in layout components
- extracted config and logic

Link to original ticket closes #2068

Link to change (deep-link for reviewing the change)

Screenshot of changes (if applicable)

DoD guidelines

[ ] user documentation added?
[ ] stakeholders approve changes? (if needed)

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 8be6e259217b36de4b8afe7365168f78f2672098

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

| Name | Type | | ----------------------------------------------- | ----- | | @commercetools-website/self-learning-smoke-test | Minor | | @commercetools-docs/gatsby-theme-docs | Minor | | @commercetools-website/api-docs-smoke-test | Patch | | @commercetools-website/docs-smoke-test | Patch | | @commercetools-website/documentation | Patch | | @commercetools-website/site-template | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions[bot] commented 2 months ago

🚚 Build Process Details

Preview URLs:

https://docskit-cc-searchbox-is-not-aligned-to-the-index-nav-width-o.commercetools.vercel.app (permalink to the last build of this branch)
https://docskit-sha-2ce66b2542a85b28d22496c42f2b7fcb2afadd3f.commercetools.vercel.app (this commit)

ColinRosati commented 1 month ago

The refactor seems ok, I'm relying mostly on VRT to ensure possible side effects. One important thing @ColinRosati make sure to link the correct issue to the PR, as far as I can see the one linked is not related with the task

Thanks updated the link. I was relying on VRT as well. I manually tested few different layouts

ColinRosati commented 1 month ago

Hey @zbalek can you help check that this fixes the layout issue for release notes? I also did some refactor to unify layout code could you click around a few pages to see if you can spot any layout changes?

zbalek commented 1 month ago

Hey @zbalek can you help check that this fixes the layout issue for release notes? I also did some refactor to unify layout code could you click around a few pages to see if you can spot any layout changes?

Hi @ColinRosati can you please share a preview link?

ColinRosati commented 1 month ago

@zbalek Theres a few different layouts here: https://docskit-cc-searchbox-is-not-aligned-to-the-index-nav-width-o.commercetools.vercel.app/docs-smoke-test/ https://docskit-cc-searchbox-is-not-aligned-to-the-index-nav-width-o.commercetools.vercel.app/api-docs-smoke-test/

For the release notes search box I can only test that locally unfornately and will have to wait till we merge and test the changes in docs release

zbalek commented 1 month ago

@ColinRosati Okay, in that case I only took a look at the layouts in the links you've shared. The second one actually misses a search box completely (?🤔)

ColinRosati commented 1 month ago

@zbalek from what I see if we compare against main on that one route the search box is not present.

Comparing https://docs-kit.commercetools.vercel.app/api-docs-smoke-test/layouts/type-regular-desktop https://docskit-cc-searchbox-is-not-aligned-to-the-index-nav-width-o.commercetools.vercel.app/api-docs-smoke-test/layouts/type-regular-desktop

Maybe Im missing something though?

gabriele-ct commented 1 month ago

As fare as I can see, the ticket seems to refer to the release notes page which exists only on the docs repo and uses a custom layout see websites/docs/src/pages/release-notes.js in docs repo. I'd start from there.

ColinRosati commented 1 month ago

As fare as I can see, the ticket seems to refer to the release notes page which exists only on the docs repo and uses a custom layout see websites/docs/src/pages/release-notes.js in docs repo. I'd start from there.

Thats right, the docs release-notes page as well as our other pages use the layout components within docs-kit. Ive tested the changes locally for release notes page, but unfortunately wont be able to get a preview deploy of the docs repo. I would propose we move forward with the PR and do the feature QA in docs release

gabriele-ct commented 1 month ago

In this case I would proceed as follow:

merge this PR
create a new release that includes these changes
apply the new release to docs repo on a feature branch
ensure the issue with the release-notes page is gone, or work on fix for it (in the same branch)
finally ask stakeholders to verify the issue directly on the docs repo preview.
merge the docs PR

Ensure that the issue remains open as this PR it's not going to fix it directly.

@FFawzy do you agree?