Skip to content

Conversation

@kraenhansen
Copy link
Contributor

Description

This is a follow-up to #6449.

Merging this PR will:

  • Disable the pointer cursor on the right-caret icon shown on disconnected rows of the connection navigation in the sidebar.
  • Refactor the way callbacks for expanding / collapsing is passed through the tree.

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@kraenhansen kraenhansen added no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) labels Nov 15, 2024
@kraenhansen kraenhansen self-assigned this Nov 15, 2024
@kraenhansen kraenhansen changed the title chore(connections-navigation): disabled sidebar caret chore(connections-navigation): disable sidebar right-caret on disconnected connections Nov 15, 2024
@kraenhansen kraenhansen requested a review from Anemy November 15, 2024 10:33
<ExpandButton
onClick={(evt) => {
if (isExpandDisabled) return;
evt.stopPropagation();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is still desirable because what I noticed - once you connect, and click on the expanded caret to collapse the tree, the default action (to open the tab for databases list) happens instead of collapse.

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 hoping you'd share context on this. I forgot to ask directly 😬
I'll add it back and write a comment on why it's there 👍

toggleExpand();
}}
isExpanded={isExpanded}
disabled={isExpandDisabled}
Copy link
Member

Choose a reason for hiding this comment

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

Based on what Betsy's message, I was under the impression we were going to hide the caret altogether, not make it disabled: https://mongodb.slack.com/archives/G2L10JAV7/p1731623253437699?thread_ts=1731413469.751959&cid=G2L10JAV7
I feel showing it in the UI makes me want to click it. I feel we should either make it not show up at all or make it clickable to connect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsradcliffe wanted to keep them (see #6449 (comment))

I would like to preserve the chevrons.

Copy link
Member

Choose a reason for hiding this comment

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

Cool cool, fine as is then. I'll ping @bsradcliffe to see if making them clickable to connect is an option, or if we want to keep it as is with the disabled.

@kraenhansen kraenhansen merged commit 2d7ff73 into main Dec 4, 2024
28 of 29 checks passed
@kraenhansen kraenhansen deleted the kh/disabled-sidebar-caret branch December 4, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants