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/23 08:30:14 UTC
[GitHub] [brooklyn-ui] algairim opened a new pull request #232: Draw relationship path labels with textPath, if supplied
algairim opened a new pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232
Signed-off-by: Mykola Mandra <my...@cloudsoftcorp.com>
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656975080
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
Review comment:
what about
```
let relationDataLabelsEntered = relationDataEntered.filter(d => d.label)
```
then of course change line above and 801 below to
```
relationDataLabelsEntered.insert('text') ...
```
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656941036
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
Review comment:
It is an heuristic number :), arch starts at the centre of the source node but ends at target's perimeter. 59% roughly reflects `middle of the arch` minus `node radius`. I'll add a comment in the code.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656937834
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
Review comment:
why `59%` ? seems curious and arbitrary. at minimum deserves an explanation. i noticed some of the long labels seemed to go close to the end of the arc. if there is a way to actually center it that would be nicer -- but if that's hard (which i assume it is) than a heuristic magic number such as this makes sense.
agree it looks nicest if it's approximately centered in the usual short-text case. starting the text near the source of the arc would make more logical sense if it has to be left-aligned but definitely wouldn't be as aesthetically pleasing. (so what you've done might be the best reasonable solution.)
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656937351
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (' ' + d.label + ' '));
Review comment:
No need to use bold, spaces are required to match the length of underneath background, created in the step above with `.html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));`
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866699984
> is it worth adding labels for the config-key relationships?
Yes, I would add those for config-key relationships, however, we need to decide what to add as a label. Is it just a name of the config key?
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656935421
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
Review comment:
instead of hiding is there a way to filter the `relationDataEntered` based on the value of `d.label` -- so we don't generate this block at all? (i assume empty text paths don't contribute much to efficiency but it might help some with large blueprints.)
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866726949
@algairim yes, the name of the config key is the right thing to show for config i think
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866801170
> Added levels for config keys of type Object [b8daa4e](https://github.com/apache/brooklyn-ui/commit/b8daa4ec8eaec3e1b733afca194f82d8511a271c), others TODO.
Addressed others.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656937351
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (' ' + d.label + ' '));
Review comment:
No need to use bold, spaces are required to match the length of underneath background, created in the step above `.html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));`
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656898649
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
Review comment:
This step might look redundant, however, this is how middle part of the path is erased for the label text:
<img width="721" alt="Screenshot 2021-06-23 at 09 55 22" src="https://user-images.githubusercontent.com/81319331/123068027-6f2dd180-d409-11eb-9996-6103b17ab9ea.png">
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656941036
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
Review comment:
It is an heuristic number :), arch starts at the centre of the source node but ends at target's perimeter. 59% roughly reflects `middle of the arch` minus node `radius`. I'll add a comment in the code.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656992509
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (' ' + d.label + ' '));
Review comment:
Removed null-check in 8d4f330fbd6c724caf36cc440fff867147a7c7c8, filter addresses that.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim edited a comment on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866745093
> @algairim yes, the name of the config key is the right thing to show for config i think
@ahgittin, we can add labels for config keys in a separate PR, requires a little bit of work.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656948122
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (' ' + d.label + ' '));
Review comment:
Null-check added in 66699f208f11c34192d7a7cc71cdd2137dfa88e9
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656900889
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
Review comment:
This step might look redundant, however, this is how middle part of the path is erased for the label text:
<img width="495" alt="Screenshot 2021-06-23 at 09 59 42" src="https://user-images.githubusercontent.com/81319331/123068491-d9467680-d409-11eb-8922-13c6079d0412.png">
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656934609
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (' ' + d.label + ' '));
Review comment:
does this get html-escaped? try with a label eg `<b>not bold</b>`. we have `import _ from lodash` available so can use that if we need to escape it.
also do we want a check `(d.label ? ' ' + d.label + ' ' : '')` here? (i know it's hidden but still feels like good practice.)
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866777705
Added levels for config keys of type Object b8daa4ec8eaec3e1b733afca194f82d8511a271c, others TODO.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866696134
is it worth adding labels for the config-key relationships?
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656898649
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
Review comment:
This step might look redundant, however, this is how middle part of the path is erased on the label text:
<img width="721" alt="Screenshot 2021-06-23 at 09 55 22" src="https://user-images.githubusercontent.com/81319331/123068027-6f2dd180-d409-11eb-9996-6103b17ab9ea.png">
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656945430
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
Review comment:
I need help with this, I was looking into how to achieve this, `hidden` attribute is a workaround.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866745093
> @algairim yes, the name of the config key is the right thing to show for config i think
@ahgittin, we can add labels to config keys in a separate PR, requires a little bit of work.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656987714
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
Review comment:
Addressed in fdc24e503da10ea2f65ce4f06e3bda7e26dfe6cb
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866695888
looks cool. a few minor comments. not tested yet -- there might be performance implications but we can address those later (and users can disable these labels if they wish).
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866644375
The label text strictly follows the arch, it can be displayed upside-down, depends on arch position. Ideas on how to address that are welcome.
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#issuecomment-866819237
Looks great. For future reference `Object.entries(entity.config).forEach( ([key, config]) => ... )` would be simpler. But merging!
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656945430
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .attr('fill', '#f5f6fa')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
+ .attr('xlink:href', (d)=>('#' + d.source._id + '-' + d.target._id))
+ .attr('startOffset', '59%')
+ .html((d) => (d.label ? '█'.repeat(d.label.length + 2) : null));
+ relationDataEntered.insert('text') // Add label text on top of '█'s which is on top of the path.
+ .attr('dominant-baseline', 'middle')
+ .attr('text-anchor', 'middle')
+ .attr('font-family', 'monospace')
+ .insert('textPath')
+ .attr('hidden', (d) => (d.label ? null : ''))
Review comment:
I need help with this, I was looking how to achieve this, `hidden` attribute is a workaround.
--
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 closed pull request #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232
--
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 #232: Draw relationship path labels with textPath, if supplied
Posted by GitBox <gi...@apache.org>.
algairim commented on a change in pull request #232:
URL: https://github.com/apache/brooklyn-ui/pull/232#discussion_r656987230
##########
File path: ui-modules/blueprint-composer/app/components/util/d3-blueprint.js
##########
@@ -776,11 +776,39 @@ export function D3Blueprint(container, options) {
let relationData = _relationGroup.selectAll('.relation')
.data(_d3DataHolder.visible.relationships, (d)=>(d.source._id + '_related_to_' + d.target._id));
- relationData.enter().insert('path')
+ let relationDataEntered = relationData.enter();
+
+ // Draw the relationship path
+ relationDataEntered.insert('path')
.attr('class', 'relation')
+ .attr('id', (d)=>(d.source._id + '-' + d.target._id))
.attr('opacity', 0)
.attr('from', (d)=>(d.source._id))
.attr('to', (d)=>(d.target._id));
+
+ // Draw the relationship label that follows the path, somewhere in the middle.
+ // NOTE `textPath` DECREASES THE UI PERFORMANCE, USE LABELS WITH CAUTION.
+ relationDataEntered.insert('text') // Add text layer of '█'s to erase the area on the path.
Review comment:
This works, fdc24e503da10ea2f65ce4f06e3bda7e26dfe6cb
--
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