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

Fix issues in new connection panel #6664

Conversation

raksooo
Copy link
Member

@raksooo raksooo commented Aug 22, 2024

  • Improve math for calculating whether or not an indicator should show.
  • Fix connection panel closing without any actions
  • Defer making feature indicators visible to ensure elements rendered

This change is Reviewable

Copy link

linear bot commented Aug 22, 2024

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @raksooo)


gui/src/renderer/components/main-view/FeatureIndicators.tsx line 210 at r1 (raw file):

  const lineIndex = Math.round((indicatorRect.top - containerRect.top) / (LINE_HEIGHT + GAP));

  return lineIndex > 1 || (lineIndex === 1 && indicatorRect.right >= ellipsisSpacerRect.left);

Nit I kind of miss the old comment explaining that an indicator should be hidden if it overlaps the spacer ellipsis. Consider re-adding some version of the old comment 😊

Code quote:

  return lineIndex > 1 || (lineIndex === 1 && indicatorRect.right >= ellipsisSpacerRect.left);

@raksooo raksooo force-pushed the feature-indicators-invisible-in-collapsed-connection-panel-des-1162 branch from a0bdbe3 to 1100d0f Compare August 29, 2024 09:59
Copy link
Member Author

@raksooo raksooo left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


gui/src/renderer/components/main-view/FeatureIndicators.tsx line 210 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Nit I kind of miss the old comment explaining that an indicator should be hidden if it overlaps the spacer ellipsis. Consider re-adding some version of the old comment 😊

Done.

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@raksooo raksooo force-pushed the feature-indicators-invisible-in-collapsed-connection-panel-des-1162 branch from 1100d0f to ff6e930 Compare August 29, 2024 13:06
@raksooo raksooo merged commit d72799b into main Aug 29, 2024
9 checks passed
@raksooo raksooo deleted the feature-indicators-invisible-in-collapsed-connection-panel-des-1162 branch August 29, 2024 13:10
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 this pull request may close these issues.

2 participants