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

chore(documentation-site): NO-JIRA enhance sidebar #406

Merged
merged 12 commits into from
Jul 17, 2024

Conversation

juliodialpad
Copy link
Collaborator

Enhance sidebar

Obligatory GIF (super important!)

Obligatory GIF

🛠️ Type Of Change

These types will not increment the version number, but will still deploy to documentation site on release:

  • Documentation
  • Build system
  • CI
  • Style (code style changes, not css changes)
  • Test
  • Other (chore)

📖 Description

  • Enhanced the sidebar with recursive capacities to be able to nest children within children.
  • Moved /content to /guides/content.
  • Added the ability to expand/collapse categories.

💡 Context

Nested categories requested by Francis.

📝 Checklist

For all PRs:

  • I have ensured no private Dialpad links or info are in the code or pull request description (Dialtone is a public repo!).
  • I have reviewed my changes.
  • I have considered the performance impact of my change.

📷 Screenshots / GIFs

@juliodialpad juliodialpad self-assigned this Jul 11, 2024
@juliodialpad juliodialpad added no-visual-test Add this tag when the PR does not need visual testing and removed no-visual-test Add this tag when the PR does not need visual testing labels Jul 11, 2024
@francisrupert
Copy link
Contributor

Awesome! Good use of Collapsible.

I'll list some comments below as individual comments rather than one long one:

@francisrupert
Copy link
Contributor

The Content page should have its own grid of child items listed too, like the attached faked screenshot below.

Those same items should not appear on the Guides landing page.

image

@francisrupert
Copy link
Contributor

francisrupert commented Jul 11, 2024

When arriving on the Guides page, the Content item should remain collapsed.

@francisrupert
Copy link
Contributor

francisrupert commented Jul 11, 2024

The Content left-nav item should be a single target. Clicking anywhere on the row expands and navigates to that page, it does not collapse.

@francisrupert
Copy link
Contributor

The "Content" item should be same text style as its peers.

image

@francisrupert
Copy link
Contributor

At the moment, we only want items that have children to be a Collapsible. That is, only "Content" on Guides should be collapsible.

The following left-nav titles should not be collapsible:

As an example, arriving on Guides should appears like so...

image

@hynes-dialpad
Copy link
Member

I agree with @francisrupert. Other than that, this LGTM!

Copy link
Contributor

@francisrupert francisrupert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Officially marking as "request changes", refer to standalone comments above.

@francisrupert
Copy link
Contributor

Remove the d-pl8 from the first level. It should only be there for the nested one.

👎 👍
image image

@francisrupert
Copy link
Contributor

francisrupert commented Jul 12, 2024

On Guides' landing page, only the items seen in Guides' leftnav should appear as a card.

image

@francisrupert
Copy link
Contributor

The Content section should be collapsed if the current page is not within Content section.

Screen.Recording.2024-07-12.at.3.46.01.PM.mov

@francisrupert
Copy link
Contributor

The leftnav should not expand when arriving at a page. Perhaps this is a limitation of DtCollapsible, but it appears to be collapsed onload and then expand onload.

Screen.Recording.2024-07-12.at.3.48.47.PM.mov

@juliodialpad
Copy link
Collaborator Author

The leftnav should not expand when arriving at a page. Perhaps this is a limitation of DtCollapsible, but it appears to be collapsed onload and then expand onload.

Screen.Recording.2024-07-12.at.3.48.47.PM.mov

Thought it'd not collapse once opened by this comment

The Content left-nav item should be a single target. Clicking anywhere on the row expands and navigates to that page, it does not collapse.

Misunderstood the behavior, fixing it 😅

Copy link

Please add either the visual-test-ready or no-visual-test label to this PR depending on whether you want to run visual tests or not.
It is recommended to run visual tests if your PR changes any UI. ‼️

@juliodialpad juliodialpad added the no-visual-test Add this tag when the PR does not need visual testing label Jul 15, 2024
Copy link
Member

@hynes-dialpad hynes-dialpad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small change here to increase the space between sections between sections on the Utilities page.

@@ -5,7 +5,7 @@
<dt-stack
v-if="sidebarItems[0]?.children.length"
as="ul"
gap="500"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the components page, there should be a gap of 500 between the sections.

Current
image

Proposed Update
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@francisrupert
Copy link
Contributor

The "Previous|Next" pagination doesn't exist. Unless it's a super quick fix, can probably just log a task to fix later.

image

@francisrupert
Copy link
Contributor

I think there's a backlog item for better RWD view of all navigation, but this ain't pretty. If you can't find a backlog item, create it.

image

@fede-dp
Copy link
Contributor

fede-dp commented Jul 15, 2024

This looks 100% better, great job!
one small comment here:
this one is bold
64f907820bb7a964e2f3429d4e57720e
and this one isn't
7d0ab10cc86f7288650694d9e91ef6e2
is this on purpose?
because if we open the main 'content' tab, that one should be bold like every other tab, right? or we keep it that way for something in particular? in their child options works fine but not in the main category.

@juliodialpad
Copy link
Collaborator Author

The "Previous|Next" pagination doesn't exist. Unless it's a super quick fix, can probably just log a task to fix later.

image

Fixed

@juliodialpad
Copy link
Collaborator Author

This looks 100% better, great job! one small comment here: this one is bold 64f907820bb7a964e2f3429d4e57720e and this one isn't 7d0ab10cc86f7288650694d9e91ef6e2 is this on purpose? because if we open the main 'content' tab, that one should be bold like every other tab, right? or we keep it that way for something in particular? in their child options works fine but not in the main category.

Thanks for catching that! Just fixed it.

@hynes-dialpad
Copy link
Member

@juliodialpad Tertiary menu items are using d-fw-semi-bold:
image

Let's remove that utility class. The indentation is sufficient IMO
image

@juliodialpad
Copy link
Collaborator Author

I think there's a backlog item for better RWD view of all navigation, but this ain't pretty. If you can't find a backlog item, create it.

image

Found a ticket but not quite sure it's exactly what you're mentioning: https://dialpad.atlassian.net/browse/DLT-895

Screenshot 2024-07-15 at 8 12 00 p m

I did this fix in the meantime.

@juliodialpad
Copy link
Collaborator Author

@juliodialpad Tertiary menu items are using d-fw-semi-bold: image

Let's remove that utility class. The indentation is sufficient IMO image

Just pushed a fix that might address that.

@hynes-dialpad
Copy link
Member

@juliodialpad I still was seeing the font weight so I went ahead and make the corrections myself. Please review and let me know.

Copy link

✔️ Deploy previews ready!
😎 Dialtone preview: https://dialtone.dialpad.com/deploy-previews/pr-406/

@hynes-dialpad hynes-dialpad dismissed francisrupert’s stale review July 17, 2024 01:17

Josh approved it and Francis is OOO. Everything looks LGTM

@juliodialpad juliodialpad merged commit 61cf94a into ux-content-guidelines-v2 Jul 17, 2024
5 checks passed
@juliodialpad juliodialpad deleted the sidebar-headers branch July 17, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-visual-test Add this tag when the PR does not need visual testing
Projects
None yet
4 participants