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

Can't interact with edges in the default node layout #8938

Closed
2 tasks
kazcw opened this issue Feb 1, 2024 · 5 comments · Fixed by #9344
Closed
2 tasks

Can't interact with edges in the default node layout #8938

kazcw opened this issue Feb 1, 2024 · 5 comments · Fixed by #9344
Assignees
Labels
--bug Type: bug -gui d-easy Difficulty: little prior knowledge required p-medium Should be completed in the next few sprints
Milestone

Comments

@kazcw
Copy link
Contributor

kazcw commented Feb 1, 2024

Discord username

No response

What type of issue is this?

Permanent – Occurring repeatably

Is this issue blocking you from using Enso?

  • Yes, I can't use Enso because of this issue.

Is this a regression?

  • Yes, previous version of Enso did not have this issue.

What issue are you facing?

When nodes are rendered in the automatic layout used if they are loaded from a file without any position metadata, they are close enough together that the nodes' selection areas completely cover the gaps between nodes. Since node selection areas take I/O precedence over edges (is this the bug here?) it is not possible to disconnect edges without rearranging nodes.

Expected behaviour

It should be possible to hover and disconnect edges even between nodes that are close together.

How we can reproduce it?

Load any file without metadata, after #8893 (which fixes a bug where automatic node layout was upside-down, so all edges were backward edges)--or move some nodes close together vertically.

Screenshots or screencasts

image

Logs

No response

Enso Version

current

Browser or standalone distribution

Chromium

Browser Version or standalone distribution

any

Operating System

MacOS

Operating System Version

No response

Hardware you are using

No response

@farmaazon
Copy link
Contributor

Yes, I think the edges should take precedence over the selection hover.

@farmaazon farmaazon added p-medium Should be completed in the next few sprints d-easy Difficulty: little prior knowledge required and removed triage labels Feb 6, 2024
@somebody1234
Copy link
Collaborator

somebody1234 commented Feb 23, 2024

i think this might be quite difficult, as selection hover is drawn over edges, right?
i wonder if there are any nice solutions

@kazcw
Copy link
Contributor Author

kazcw commented Feb 23, 2024

i think this might be quite difficult, as selection hover is drawn over edges, right? i wonder if there are any nice solutions

It's drawn over edges, but we actually already render edges twice--the visible version, and a visually-transparent IO version (with a wider stroke width); so I think we can fix this by simply ensuring the IO layer is above the selection.

@farmaazon farmaazon added this to the Beta Release milestone Feb 23, 2024
@farmaazon
Copy link
Contributor

farmaazon commented Feb 27, 2024

Refinement notes:

  • Selection areas need to be in a separate layer, under all nodes and edges (same for "interaction area" of selection).
  • The selection may be teleported to their layer, or just handled separately (in kind of GraphSelections component).

@kazcw kazcw self-assigned this Mar 8, 2024
@kazcw kazcw linked a pull request Mar 8, 2024 that will close this issue
5 tasks
@enso-bot
Copy link

enso-bot bot commented Mar 8, 2024

Keziah Wesley reports a new STANDUP for today (2024-03-08):

Progress: Addressed review on some PRs; fixed selection layering. It should be finished by 2024-03-08.

Next Day: Next day I will be working on the #8938 task. Pick something from backlog.

@mergify mergify bot closed this as completed in #9344 Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -gui d-easy Difficulty: little prior knowledge required p-medium Should be completed in the next few sprints
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants