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

[Enhancement]: Path inspector left and right-click suggestions #348

Closed
Gregg8 opened this issue Jan 21, 2022 · 7 comments · Fixed by #356
Closed

[Enhancement]: Path inspector left and right-click suggestions #348

Gregg8 opened this issue Jan 21, 2022 · 7 comments · Fixed by #356
Milestone

Comments

@Gregg8
Copy link
Contributor

Gregg8 commented Jan 21, 2022

1 - Right-click menu improvements

Suggestions:

  • Would love to see an actual right-click popup menu rather than one that embeds itself into the UI.
  • Until that comes (if it comes), I have the following suggestions for the embedded version:
    • The width of the menu currently extends across the max width of the inspector (which can be much wider than the window). Because the menu text is centred, you often can't even see it. So, just give it a fixed width
    • It would be nice if the cursor was set to a hand pointer while hovering over the buttons

2 - New path replaces current path

It's quite jarring to have the parent tree disappear when you click on items.

After some discussions, we've decided on four functions that should cover all use cases:

Open path in current inspector

  • Double-click on the member keyword (or the value where appropriate)
  • Previously this was done with a single-click

Open path in NEW inspector

  • Single-middle-click on the member keyword (or the value where appropriate)
  • It's similar to browsers, which open a url in a new tab on a middle-click

Open parent path in current inspector

  • Add a small md-icon-button next to the path input-text
  • The icon should be an up-arrow
  • When clicked, it modifies the current path to be the parent
  • It does this by popping off the last item in the path
  • When it gets to the top level, it should also remove the empty square brackets. i.e. []
  • When there is no parent available, it should be disabled

Clear inspector path

  • Add another small md-icon-button next to the path input-text
  • The icon should be an X
  • When clicked, it simply clears the path text

See the screenshot below for button placement:

  • Feel free to place them next to the input text or inside it
  • Next to is easier to implement

image

@MawiraIke
Copy link
Contributor

MawiraIke commented Feb 21, 2022

The tasks from this issue are:

  • Create an actual right click instead of embedded popup
  • Change option to open path in current inspector to use double click
  • Add option to open new path in new inspector
  • Add button to open the parent path in the current inspector
  • Add button to clear the path

@MawiraIke MawiraIke self-assigned this Feb 21, 2022
@Gregg8
Copy link
Contributor Author

Gregg8 commented Feb 22, 2022

@MawiraIke I've updated the spec for item 2 above ^^^

Note: we're skipping the "history of paths with step back and forward functionalities" feature

@MawiraIke
Copy link
Contributor

Got it @Gregg8 .
I have updated the PR #350 with the new features.

@Gregg8
Copy link
Contributor Author

Gregg8 commented Mar 17, 2022

Nice improvements. Love the parent path and clear path buttons.

Some feedback:

  • The right-click menu appears about 700px to the right for me.
    • The red arrow is pointing to where I right-clicked:
      image
  • Middle-click to open the path in a new inspector is great. A couple of suggested improvements:
    • In Chrome, this changes (not all the time, not sure of the exact circumstances) the UI to scroll mode (and the cursor changes to a scrolling cursor). Need to click away to get out of it. I believe this could be prevented with a preventDefault call at the end of the click event handler
    • The inspector is closed by default. Open would be better, I think

@MawiraIke
Copy link
Contributor

Thanks @Gregg8 for the feedback. The issues you mentioned above are in continuous development and the second one regarding middle click triggering scroll mode should be fixed in the latest release 1.2.4.
I will update on the rest here after committing their fixes.

@MawiraIke
Copy link
Contributor

@Gregg8 the remaining two issues should be fixed on PR #356 waiting to be merged.
That is:

  1. The right-click menu appears about 700px to the right. This should be fixed and the menu should show to the immediate right of the mouse click - as if anchored by its top left corner.
  2. The inspector is closed by default. I have added an option in settings that allows users to choose the default behavior that will affect all new inspectors created.

@Gregg8
Copy link
Contributor Author

Gregg8 commented Apr 20, 2022

Tested 1.2.6:

  1. Thanks. This is much better than before. However, instead of anchoring the popup menu to the position of the mouse pointer, it is anchored to the top-left corner of the object being clicked on, so if you click at the beginning of an object, it's sufficiently close the the mouse-click, but if you click near the end of a long object name, it appears quite far to the left of the mouse-click. In the image below, the arrow points to the caret, which is where I clicked...
    image
  2. Great

@Gregg8 Gregg8 reopened this Apr 27, 2022
@superstructor superstructor added this to the v1.6.0 milestone Jul 5, 2022
kimo-k added a commit that referenced this issue Jul 16, 2023
Added mouse-offset parameters. Now, the menu appears at the exact
mouse position.

I'm not sure what the importance of popup-menus is. Maybe it's just a
kind of render memoization, build-popup doesn't seem expensive enough
to justify it.

Removed this pattern. Such memoization doesn't really work here, since
the new mouse-offset tends to be a different value every time.

#348
kimo-k added a commit that referenced this issue Jul 16, 2023
Added mouse-offset parameters. Now, the menu appears at the exact
mouse position.

I'm not sure what the importance of popup-menus is. Maybe it's just a
kind of render memoization, but build-popup doesn't seem expensive
enough to justify it.

Removed this pattern. Such memoization doesn't really work here, since
the new mouse-offset tends to be a different value every time.

#348
kimo-k added a commit that referenced this issue Jul 26, 2023
Added mouse-offset parameters. Now, the menu appears at the exact
mouse position.

I'm not sure what the importance of popup-menus is. Maybe it's just a
kind of render memoization, but build-popup doesn't seem expensive
enough to justify it.

Removed this pattern. Such memoization doesn't really work here, since
the new mouse-offset tends to be a different value every time.

#348
@kimo-k kimo-k closed this as completed Jul 26, 2023
kimo-k added a commit that referenced this issue Nov 30, 2023
kimo-k added a commit that referenced this issue Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants