You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/06/18 21:23:47 UTC
[GitHub] [brooklyn-ui] algairim opened a new pull request #227: WIP Drag and drop enhancements
algairim opened a new pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim edited a comment on pull request #227: Dra & drop improvements
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
TODO:
- [x] Add new tests for multi-selection mode
- [x] Pre-select checkbox if there is one option provided in multi-selection mode only.
- [x] Disable 'Apply' button in multi-selection mode until at least one option is ticked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865245685
fine to merge as is but will wait for reply or user-click-elsewhere if wanted in this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim edited a comment on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
TODO:
- [x] Add new tests for multi-selection mode
- [x] Pre-select checkbox if there is one option provided in multi-selection mode only.
- [x] Disable 'Apply' button in multi-selection mode until at least one option is ticked.
- [ ] if user clicks elsewhere - the menu goes away.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim commented on pull request #227: Dra & drop improvements
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865818461
Agreed to implement "if user clicks elsewhere - the menu goes away" in a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on a change in pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#discussion_r655605319
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -1097,25 +1097,53 @@ export function D3Blueprint(container, options) {
/**
* Draw menu with a confirmation request on the canvas for a node with a specified ID, under the node, in the middle.
+ *
+ * Single-selection mode offers choices as buttons:
* _________________________
* | Confirmation message |X|| <- 'close' button
* | ________ |
- * ||Choice 1| |
+ * ||Choice 1| | <- e.g. Button with confirmation choice 'Choice 1'
* | --------' |
* ||Choice 2| |
* | --------' |
* ||Etc. | |
* '-------------------------'
*
+ * Multi-selection mode offers choices as check-boxes and confirmation button.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * || | Choice 1 | <- e.g. Checkbox with confirmation choice 'Choice 1'
+ * | - |
+ * || | Choice 2 |
+ * | - |
+ * || | Etc. |
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choices.
+ * '-------------------------'
+ *
+ * Confirmation choice is pre-selected for a single option in multi-selected mode.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * ||X| Choice 1 | <- Checkbox with pre-selected confirmation choice 'Choice 1'
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choice.
+ * '-------------------------'
+ *
+ * Apply button is disabled until at least one choice is selected in the multi-selection mode.
+ *
* @param {String} id The ID of a node to draw confirmation menu for.
* @param {String} message The confirmation message.
- * @param {Array.<String>} choices The confirmation choices.
- * @param {Function} callback The confirmation callback with a boolean parameter, true if confirmed (Yes),
- * false if denied (No), null otherwise (ignored).
+ * @param {Array.<String|Object>} choices The confirmation choices as plain strings or objects that implement toString
+ * to display choice options. Use objects with distinct *.id value for choices that return same toString value.
+ * @param {Function} callback The callback with confirmed choice in the single-selection mode, or with {Array.<String>}
+ * of confirmed choices in the multi-selection mode.
+ * @param {Boolean} isMultiSelection Indicates if confirmation selection is multi-selection, single-selection is default.
Review comment:
should `default` of type `<String|Object>` (or `Array` thereof for `isMultiSelection`) be a parameter?
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -1097,25 +1097,53 @@ export function D3Blueprint(container, options) {
/**
* Draw menu with a confirmation request on the canvas for a node with a specified ID, under the node, in the middle.
+ *
+ * Single-selection mode offers choices as buttons:
* _________________________
* | Confirmation message |X|| <- 'close' button
* | ________ |
- * ||Choice 1| |
+ * ||Choice 1| | <- e.g. Button with confirmation choice 'Choice 1'
* | --------' |
* ||Choice 2| |
* | --------' |
* ||Etc. | |
* '-------------------------'
*
+ * Multi-selection mode offers choices as check-boxes and confirmation button.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * || | Choice 1 | <- e.g. Checkbox with confirmation choice 'Choice 1'
+ * | - |
+ * || | Choice 2 |
+ * | - |
+ * || | Etc. |
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choices.
+ * '-------------------------'
+ *
+ * Confirmation choice is pre-selected for a single option in multi-selected mode.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * ||X| Choice 1 | <- Checkbox with pre-selected confirmation choice 'Choice 1'
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choice.
+ * '-------------------------'
+ *
+ * Apply button is disabled until at least one choice is selected in the multi-selection mode.
+ *
* @param {String} id The ID of a node to draw confirmation menu for.
* @param {String} message The confirmation message.
- * @param {Array.<String>} choices The confirmation choices.
- * @param {Function} callback The confirmation callback with a boolean parameter, true if confirmed (Yes),
- * false if denied (No), null otherwise (ignored).
+ * @param {Array.<String|Object>} choices The confirmation choices as plain strings or objects that implement toString
+ * to display choice options. Use objects with distinct *.id value for choices that return same toString value.
+ * @param {Function} callback The callback with confirmed choice in the single-selection mode, or with {Array.<String>}
+ * of confirmed choices in the multi-selection mode.
+ * @param {Boolean} isMultiSelection Indicates if confirmation selection is multi-selection, single-selection is default.
Review comment:
(so callers can specify what is initially selected?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #227: Dra & drop improvements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865841829
there are known bugs with foreignobject on safari which appear to be causing the problems there:
* https://bugs.webkit.org/show_bug.cgi?id=90738 (has patch)
* https://bugs.webkit.org/show_bug.cgi?id=23113 (depends on above; might be fixed by patch?)
as there are recent patches i think best thing to do is wait for them to land in safari and not worry about those issues for now. it would need a major rework to abandon foreignobject and is not worth it IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #227: Dra & drop improvements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865820579
works great. no need to allow defaults to be configured just yet -- that can come. some minor errors and remaining tasks:
* popup goes away if somewhere else is clicked
* issue w z-order meaning parts of the button are not clickable in some cases
* issue w <path attribute "d" incomplete when nodes are moved
* issue on safari where buttons don't appear
* issue after relationship created that the arc doesn't appear
but merging as it's so good; remaining tasks to be addressed/considered in follow-up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim edited a comment on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865245425
looks great, but for one comment on allowing default values to be passed in
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim edited a comment on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim edited a comment on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
TODO:
- [ ] Add new tests for multi-selection mode
- [x] Pre-select checkbox if there is one option provided in multi-selection mode only.
- [x] Disable 'Apply' button in multi-selection mode until at least one option is ticked.
- [ ] if user clicks elsewhere - the menu goes away.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim commented on a change in pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#discussion_r654557655
##########
File path: ui-modules/blueprint-composer/app/components/designer/designer.directive.js
##########
@@ -203,38 +204,46 @@ export function designerDirective($log, $state, $q, iconGenerator, catalogApi, b
$element.bind('drop-external-node', event => {
let draggedItem = paletteDragAndDropService.draggedItem();
- let target = blueprintService.find(event.detail.parentId);
+ let targetEntity = blueprintService.find(event.detail.parentId);
if (draggedItem.supertypes.includes(EntityFamily.ENTITY.superType)) {
let targetIndex = event.detail.targetIndex;
let newEntity = blueprintService.populateEntityFromApi(new Entity(), draggedItem);
if (targetIndex >= 0) {
- target.insertChild(newEntity, targetIndex);
+ targetEntity.insertChild(newEntity, targetIndex);
} else {
- target.addChild(newEntity);
+ targetEntity.addChild(newEntity);
}
blueprintService.refreshEntityMetadata(newEntity, EntityFamily.ENTITY).then(() => {
+ container.dispatchEvent(new CustomEvent('new-entity-created', {
+ detail: {
+ nodeId: newEntity._id,
+ parentId: targetEntity._id
+ }
+ }));
Review comment:
Consistent with 'move-entity' event.
##########
File path: ui-modules/blueprint-composer/app/components/designer/designer.directive.js
##########
@@ -203,38 +204,46 @@ export function designerDirective($log, $state, $q, iconGenerator, catalogApi, b
$element.bind('drop-external-node', event => {
let draggedItem = paletteDragAndDropService.draggedItem();
- let target = blueprintService.find(event.detail.parentId);
+ let targetEntity = blueprintService.find(event.detail.parentId);
if (draggedItem.supertypes.includes(EntityFamily.ENTITY.superType)) {
let targetIndex = event.detail.targetIndex;
let newEntity = blueprintService.populateEntityFromApi(new Entity(), draggedItem);
if (targetIndex >= 0) {
- target.insertChild(newEntity, targetIndex);
+ targetEntity.insertChild(newEntity, targetIndex);
} else {
- target.addChild(newEntity);
+ targetEntity.addChild(newEntity);
}
blueprintService.refreshEntityMetadata(newEntity, EntityFamily.ENTITY).then(() => {
+ container.dispatchEvent(new CustomEvent('new-entity-created', {
+ detail: {
+ nodeId: newEntity._id,
+ parentId: targetEntity._id
+ }
+ }));
Review comment:
`detail` is consistent with 'move-entity' event.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on a change in pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#discussion_r655605656
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -1097,25 +1097,53 @@ export function D3Blueprint(container, options) {
/**
* Draw menu with a confirmation request on the canvas for a node with a specified ID, under the node, in the middle.
+ *
+ * Single-selection mode offers choices as buttons:
* _________________________
* | Confirmation message |X|| <- 'close' button
* | ________ |
- * ||Choice 1| |
+ * ||Choice 1| | <- e.g. Button with confirmation choice 'Choice 1'
* | --------' |
* ||Choice 2| |
* | --------' |
* ||Etc. | |
* '-------------------------'
*
+ * Multi-selection mode offers choices as check-boxes and confirmation button.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * || | Choice 1 | <- e.g. Checkbox with confirmation choice 'Choice 1'
+ * | - |
+ * || | Choice 2 |
+ * | - |
+ * || | Etc. |
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choices.
+ * '-------------------------'
+ *
+ * Confirmation choice is pre-selected for a single option in multi-selected mode.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * ||X| Choice 1 | <- Checkbox with pre-selected confirmation choice 'Choice 1'
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choice.
+ * '-------------------------'
+ *
+ * Apply button is disabled until at least one choice is selected in the multi-selection mode.
+ *
* @param {String} id The ID of a node to draw confirmation menu for.
* @param {String} message The confirmation message.
- * @param {Array.<String>} choices The confirmation choices.
- * @param {Function} callback The confirmation callback with a boolean parameter, true if confirmed (Yes),
- * false if denied (No), null otherwise (ignored).
+ * @param {Array.<String|Object>} choices The confirmation choices as plain strings or objects that implement toString
+ * to display choice options. Use objects with distinct *.id value for choices that return same toString value.
+ * @param {Function} callback The callback with confirmed choice in the single-selection mode, or with {Array.<String>}
+ * of confirmed choices in the multi-selection mode.
+ * @param {Boolean} isMultiSelection Indicates if confirmation selection is multi-selection, single-selection is default.
Review comment:
(so callers can specify what is initially selected?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on a change in pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#discussion_r655605319
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -1097,25 +1097,53 @@ export function D3Blueprint(container, options) {
/**
* Draw menu with a confirmation request on the canvas for a node with a specified ID, under the node, in the middle.
+ *
+ * Single-selection mode offers choices as buttons:
* _________________________
* | Confirmation message |X|| <- 'close' button
* | ________ |
- * ||Choice 1| |
+ * ||Choice 1| | <- e.g. Button with confirmation choice 'Choice 1'
* | --------' |
* ||Choice 2| |
* | --------' |
* ||Etc. | |
* '-------------------------'
*
+ * Multi-selection mode offers choices as check-boxes and confirmation button.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * || | Choice 1 | <- e.g. Checkbox with confirmation choice 'Choice 1'
+ * | - |
+ * || | Choice 2 |
+ * | - |
+ * || | Etc. |
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choices.
+ * '-------------------------'
+ *
+ * Confirmation choice is pre-selected for a single option in multi-selected mode.
+ * _________________________
+ * | Confirmation message |X||
+ * | _ |
+ * ||X| Choice 1 | <- Checkbox with pre-selected confirmation choice 'Choice 1'
+ * | -------- |
+ * ||Apply | | <- Button to apply confirmed choice.
+ * '-------------------------'
+ *
+ * Apply button is disabled until at least one choice is selected in the multi-selection mode.
+ *
* @param {String} id The ID of a node to draw confirmation menu for.
* @param {String} message The confirmation message.
- * @param {Array.<String>} choices The confirmation choices.
- * @param {Function} callback The confirmation callback with a boolean parameter, true if confirmed (Yes),
- * false if denied (No), null otherwise (ignored).
+ * @param {Array.<String|Object>} choices The confirmation choices as plain strings or objects that implement toString
+ * to display choice options. Use objects with distinct *.id value for choices that return same toString value.
+ * @param {Function} callback The callback with confirmed choice in the single-selection mode, or with {Array.<String>}
+ * of confirmed choices in the multi-selection mode.
+ * @param {Boolean} isMultiSelection Indicates if confirmation selection is multi-selection, single-selection is default.
Review comment:
should `default` of type `<String|Object>` (or `Array` thereof for `isMultiSelection`) be a parameter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865245425
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] ahgittin commented on pull request #227: Dra & drop improvements
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-865855953
the z-order / button clickability bug goes away if we reverse the order of the nodes, eg d3-blueprint.js line 549 add `.reverse()`:
```
_d3DataHolder.nodes = root.descendants().reverse();
```
this is because svg layers things in the order they are listed. so without this the bottom row of buttons is logically on top of its parents `g` element. `foreignObject` seems to transcend this in some ways but not completely. painting the things bottom-first means the foreignObject is above the children nodes by all modes of reckoning.
(unfortunately this doesn't fix any of the safari issues, and i've noticed one more nasty one: sometimes the tab can hang trying to render the popup. only solution i know of is to kill and restart safari! even stronger evidence it's a nasty bug in safari, so not worth _our_ trying to fix it, unless we stumble on an easy fix, but hopefully will be fixed in safari/webkit soon!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim edited a comment on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
TODO:
- [ ] Add new tests for multi-selection mode
- [ ] Pre-select checkbox if there is one option provided in multi-selection mode only.
- [ ] Disable 'Apply' button in multi-selection mode until at least one option is ticked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] algairim commented on pull request #227: WIP Drag and drop enhancements
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227#issuecomment-864139085
- [ ] Add new tests for multi-selection mode
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-ui] asfgit merged pull request #227: Dra & drop improvements
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #227:
URL: https://github.com/apache/brooklyn-ui/pull/227
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org