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 context menu and components panel spawn coordinates #169

Merged
merged 6 commits into from
Jul 13, 2022

Conversation

AdrySky
Copy link
Contributor

@AdrySky AdrySky commented Jul 5, 2022

Description

This will fix context menu and component panel from rendering at the wrong position.

Pull Request Type

  • Xircuits Core (Jupyterlab Related changes)
  • Xircuits Canvas (Custom RD Related changes)
  • Xircuits Component Library
  • Testing Automation
  • Documentation
  • Others (Please Specify)

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Tests

  1. Open context menu and component panel
    1. Create .xircuits file
    2. Right-click to open context menu anywhere on canvas
    3. Drop a link onto canvas to open components panel
    4. Make sure both panel was opened inside canvas
    5. Also, make sure it's spawn at the correct position where the mouse was clicked or link dropped

Tested on?

  • Windows
  • Linux Ubuntu
  • Centos
  • Mac
  • Others (State here -> xxx )

@AdrySky AdrySky added the bug Something isn't working label Jul 5, 2022
@AdrySky AdrySky requested a review from MFA-X-AI July 5, 2022 02:42
@AdrySky AdrySky self-assigned this Jul 5, 2022
Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Looks like it's fixed on my end, great work! The context menu and component list spawn correctly near the click point.
updated spawn
If I had to nitpick, maybe the spawn coordinates for the most bottom location might be shifted upwards for a few pixels so users can see the, Add Comment Component option, but otherwise this is good enough already.

@AdrySky
Copy link
Contributor Author

AdrySky commented Jul 6, 2022

the spawn coordinates for the most bottom location might be shifted upwards for a few pixels so users can see the, Add Comment Component option,

Done

Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Hmm. The bottom spawn went the other direction than expected.
spawn-coor

I've tried some changes myself, it was pretty manual for my tastes but it does fix it.
suggested-spawn
https://gist.github.com/MFA-X-AI/0f4570dae5e8dccf777189a87c5670d0#file-gistfile1-txt-L1601-L1622
Let me know if there's a smarter way of doing it. 😄

@AdrySky
Copy link
Contributor Author

AdrySky commented Jul 12, 2022

Thanks for noticing this bug. Don't know why I've to add some additional offset when it height changed. Currently, it's not possible yet to get a dynamic height(Though, the width is fine idk why). The way things works is everytime adding/removing an options, the offset need changes.

Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Caught an extra bug for this one.
extra-spawn
Basically the spawn location gets thrown off when an extra xircuits canvas is open.
I think the spawn variable needs to be either exclusive for each canvas tab or coded so that it takes a fixed value.

@AdrySky
Copy link
Contributor Author

AdrySky commented Jul 12, 2022

Okayy..Was not expecting that. Good job for finding that bug.
Didn't realize a simple fix could cause such annoying bugs.

Anyway, will work on fixing that.

@AdrySky
Copy link
Contributor Author

AdrySky commented Jul 12, 2022

Figured out what causing it but not sure why... I guess it's probably because the canvas bottom area goes outside of the screen. Thus, getting its exact bottom position is a bit difficult. Right now, I just only use the top position for the y-axis.

Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Works perfectly, adding tabs, zooming in and out will have consistent spawn location. Great job!
spawn-working

@MFA-X-AI MFA-X-AI merged commit 92b7c25 into master Jul 13, 2022
@MFA-X-AI MFA-X-AI deleted the adry/fix-panel-coor branch July 13, 2022 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants