You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "samar121 (via GitHub)" <gi...@apache.org> on 2025/04/22 08:33:31 UTC

[PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

samar121 opened a new pull request, #9888:
URL: https://github.com/apache/nifi/pull/9888

   🛠️ Fix: Label rendering issue in boundedMultilineEllipsis
   This MR addresses a bug where SVG labels using boundedMultilineEllipsis were not displaying correctly when rendered via D3, especially during interactions like dragging or viewport changes.
   
   ✅ Root Cause:
   The issue stemmed from inaccurate line height and line count calculations, which affected label wrapping and positioning. In some cases, the initial tspan was not receiving the proper y attribute, causing the entire label to be misaligned or not visible.
   
   🔧 Fix Details:
   Reordered the logic inside the while (word) loop to:
   
   1. Assign y position immediately on first word append, ensuring the tspan gets positioned correctly even for single-word lines.
   2. Calculate lineHeight and lineCount using getBoundingClientRect() only once (on the first word of the first line), improving performance and consistency.
   3. Preserve leading spaces using xml:space="preserve" to match label formatting.
   4. Ensure all wrapped lines are properly positioned using dy="1.2em" and line limit logic.
   
   📷 Screenshots:
   Before (Broken Labels):
   ![Screenshot from 2025-04-21 22-23-15](https://github.com/user-attachments/assets/26fb795e-8038-4840-a535-958919e89ce7)
   
   After (Fixed Labels):
   ![Screenshot from 2025-04-22 10-02-05](https://github.com/user-attachments/assets/d3ff4ff5-2e9a-41d8-9203-26d3bc6eca69)
   
   
   🔬 Validation:
   Tested rendering behavior on:
   1. Long and short label strings
   2. Multi-line wrapping scenarios
   3. Resizing, dragging, zooming, and panning
   
   All scenarios now render correctly as expected.
   
   📌 Note:
   This fix improves layout consistency and ensures robust label handling across different canvas states and screen sizes.
   
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "samar121 (via GitHub)" <gi...@apache.org>.
samar121 commented on PR #9888:
URL: https://github.com/apache/nifi/pull/9888#issuecomment-2835496855

   Hi team,
   
   Thank you for merging the changes and mentioning me!
   My GitHub username and Apache JIRA ID are different, so the issue was not automatically assigned to me.
   
   Could you please assign NIFI-14071 to me for proper tracking?
   My Apache JIRA ID is:  samarpratap
   
   Thanks again!
   cc: @rfellows 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "samar121 (via GitHub)" <gi...@apache.org>.
samar121 commented on PR #9888:
URL: https://github.com/apache/nifi/pull/9888#issuecomment-2835164362

   Thanks for the catch @rfellows 
   I've updated the logic to only prepend leading whitespace if it actually exists, as suggested.
   This should eliminate unnecessary string operations and make the label resizing feel smoother.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #9888:
URL: https://github.com/apache/nifi/pull/9888#discussion_r2063562346


##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/service/canvas-utils.service.ts:
##########
@@ -1478,19 +1478,27 @@ export class CanvasUtils {
         let lineHeight = height;
 
         for (const fullLine of lines) {
-            const words: string[] = fullLine.split(/\s+/).reverse();
+            // Extract and preserve only the leading whitespace at the start of the line
+            const trimmedLine = fullLine.trimStart();
+            const leadingWhitespace = fullLine.slice(0, fullLine.length - trimmedLine.length);
+
+            // Split words normally, letting internal whitespace collapse
+            const words = trimmedLine.split(/\s+/).reverse();
+            if (words.length > 0) {
+                words[words.length - 1] = leadingWhitespace + words[words.length - 1]; // Prepend leading space to the first word
+            }

Review Comment:
   There is a minor performance hit here. It can make resizing of the label feel jittery. It can be fixed by adding an additional condition here to only update the array in this manor if there is actually whitespace to preserve:
   
   ```suggestion
               if (leadingWhitespace.length > 0 && words.length > 0) {
                   words[words.length - 1] = leadingWhitespace + words[words.length - 1]; // Prepend leading space to the first word
               }
   ```
   
   



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "samar121 (via GitHub)" <gi...@apache.org>.
samar121 commented on PR #9888:
URL: https://github.com/apache/nifi/pull/9888#issuecomment-2829250620

   Thanks for the feedback! I’ve now added comments back into the boundedMultilineEllipsis method to explain the reasoning behind the changes and key parts of the logic.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on PR #9888:
URL: https://github.com/apache/nifi/pull/9888#issuecomment-2835569671

   @samar121 Thanks again for the pull request! I have updated the JIRA to be assigned to you.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows commented on code in PR #9888:
URL: https://github.com/apache/nifi/pull/9888#discussion_r2060209563


##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/service/canvas-utils.service.ts:
##########
@@ -1478,11 +1478,16 @@ export class CanvasUtils {
         let lineHeight = height;
 
         for (const fullLine of lines) {
-            const words: string[] = fullLine.split(/\s+/).reverse();
+            // Preserve leading spaces per word
+            const words: string[] = fullLine.match(/(\s*\S+)/g)?.reverse() || [];

Review Comment:
   This might be too aggressive. Now, all leading whitespace is considered part of the subsequent word treating space, tab, and line breaks as regular characters and not as an opportunity to wrap text at those points. Prior to this change, whitespace was treated similar to how it renders in html, multiple consecutive spaces would render as a single one and would allow wrapping there. Now, if you have many spaces together between words the leading spaces before the word are now bound together and will not allow wrapping in the middle of the spaces, treating a string like "          hello" as 15 characters rather than 5.
   
   Consider the string `Hello                                           World`
   Before change:
   <img width="314" alt="Screenshot 2025-04-25 at 09 06 20" src="https://github.com/user-attachments/assets/1dcbc102-2e8b-49ba-88f2-884ee28eb3e3" />
   
   After change:
   <img width="276" alt="Screenshot 2025-04-25 at 09 06 51" src="https://github.com/user-attachments/assets/31873fd1-54ff-4ccf-bed8-0eca223b2a86" />
   
   I think the best solution would be to only preserve whitespace at the beginning of a line(start of string or following line break) and let whitespace collapse elsewhere like it did before.
   
   Another consequence of the current solution is that since the leading space is preserved, it can make lines look indented when they do wrap to the next line:
   <img width="556" alt="Screenshot 2025-04-25 at 09 02 54" src="https://github.com/user-attachments/assets/b6e07a73-2e67-41c0-900a-fb0504f3fdc9" />
   
   
   



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "samar121 (via GitHub)" <gi...@apache.org>.
samar121 commented on PR #9888:
URL: https://github.com/apache/nifi/pull/9888#issuecomment-2831823300

   Thanks a lot for the thoughtful feedback, @rfellows!
   You were absolutely right — the earlier change was too aggressive in preserving all leading whitespace, which unintentionally affected how the lines wrapped, especially when there were multiple spaces between words.
   I've updated the logic to preserve leading whitespace only at the beginning of each line (i.e., at the start of a string or immediately after a line break) while allowing normal whitespace collapsing between words,
   
   ✅ After the change — for a string like      Hello                                           World
   ![image](https://github.com/user-attachments/assets/0a166f1d-95d9-47a2-be5d-91a10d12ba16)
   
   ✅ Leading space preservation and unintended indentation issues are also fixed:
   ![image](https://github.com/user-attachments/assets/6384cc0f-f392-43c8-9ec5-418f4b055bfd)
   
   Please let me know if you spot anything else! 


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] NIFI-14071: Labels not rendering lines starting with spaces [nifi]

Posted by "rfellows (via GitHub)" <gi...@apache.org>.
rfellows merged PR #9888:
URL: https://github.com/apache/nifi/pull/9888


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org