You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by mcgilman <gi...@git.apache.org> on 2017/01/18 21:00:32 UTC
[GitHub] nifi pull request #1430: NIFI-3301: Fix cursor style over event type text in...
GitHub user mcgilman opened a pull request:
https://github.com/apache/nifi/pull/1430
NIFI-3301: Fix cursor style over event type text in lineage graph
NIFI-3301:
- Removing incorrect cursor style when hovering over event type text in a linage graph.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mcgilman/nifi NIFI-3301
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/1430.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1430
----
commit abcac5070b5860efafedd03c03992b6e47db04c1
Author: Matt Gilman <ma...@gmail.com>
Date: 2017-01-18T20:59:36Z
NIFI-3301:
- Removing incorrect cursor style when hovering over event type text in a linage graph.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/1430
I just pushed an update making the suggested changes. Thanks again!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by jvwing <gi...@git.apache.org>.
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1430
Reviewing...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/1430
Good call. I had my setup to ignore whitespace following my rebase and I overlooked it. I'm pretty sure it's correct now but it still looks like some things are shifted because I moved a lot of the logic into a showContextMenu function instead of handling everything within the anonymous callback.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by jvwing <gi...@git.apache.org>.
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1430
@mcgilman - I certainly don't object to this change, but I'm not convinced it resolves a lot of friction in the lineage graph. On Chrome and Firefox, removing the "pointer" style results in an I-beam cursor when hovering over the text labels. But you can't select text, clicking results in drag behavior more suitable to the "move" or "grab" cursors, and identical to the background drag behavior represented with an arrow or "default" cursor. The graph nodes get an arrow cursor on hover, but since the background has the same cursor, it doesn't visually distinguish that nodes permit a right-click interaction.
What would you think about a move/grab cursor over both the background and the text labels, with an default arrow for the nodes?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by jvwing <gi...@git.apache.org>.
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1430
@mcgilman Your argument for consistency is very persuasive, and I would agree that adding the move/grab cursor is inconsistent. A brief comparison with the flow canvas:
- Arrow/default cursor over the canvas, even when dragging
- Hand/pointer cursor over processors, suggesting right-click/ctrl-click action
So for the lineage graph, I would translate this to:
- Arrow/default with drag-to-pan behavior for the background
- Hand/pointer hovering over lineage node circles where there is a right-click/ctrl-click action
- Label text is a toss-up, which is why we're here :)
What about if we considered the entire lineage node circle + label text as a unit, sharing both the hand/pointer cursor and the right-click menu behavior. It would just be a bigger, simpler target. Would that help resolve confusion over how to interact with it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/1430
Sounds good. I've made the changes. However, I'm going to hold off on pushing the update since it'll conflict with some changes that I'm reviewing in another PR (#1428) with someone else. Once that is wrapped up, I'll handle the rebase and update this effort.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1430: NIFI-3301: Fix cursor style over event type text in...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/1430
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by jvwing <gi...@git.apache.org>.
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1430
Thanks, @mcgilman , the update looks good. I squashed and merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by jvwing <gi...@git.apache.org>.
Github user jvwing commented on the issue:
https://github.com/apache/nifi/pull/1430
@mcgilman - Thank you for making this more elaborate fix, I like the way it both looks and works. I have a question about the code diffs, though, it looks like there is a lot of whitespace-only indentation change between lines ~546 and ~1034 of `nf-provenance-lineage.js`. Is that intentional and desired?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1430: NIFI-3301: Fix cursor style over event type text in lineag...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/1430
@jvwing Thanks for the prompt review!
I certainly don't mind having a more comprehensive review of the current behavior. The initial stab at the PR was simply trying to address the confusing bit reported surrounding the incorrect hand pointer over the event text.
We should probably consider updating the main canvas as well to ensure its consistent throughout the application. In the context of the canvas, panning is supported by clicking and dragging on the background and moving is supported by clicking and dragging on a component. I'm thinking we should leave the arrow cursor in place on the canvas (as it's currently implemented) and introduce the move cursor when dragging (moving a component and panning only). This behavior would be similar to other tools like Google Maps.
Carrying this behavior into the lineage graph, we could always have the arrow pointer (like the canvas) and clicking the background would pan and show the move cursor. Additionally, I think we should prevent panning if the mousedown happens on a lineage node or event type text.
Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---