[GitHub] [ofbiz-framework] danwatford commented on a diff in pull request #547: Improved: Support title attribute on hyperlinks (OFBIZ-12697)

[GitBox <gi...@apache.org>]

danwatford commented on code in PR #547:
URL: https://github.com/apache/ofbiz-framework/pull/547#discussion_r988765575


##########
framework/widget/src/test/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRendererTest.java:
##########
@@ -887,6 +887,55 @@ public void renderSortFieldUsesQueryString(@Mocked ModelForm modelForm) throws I
                 "linkUrl", new FreemarkerRawString(linkFromQbeString)));
     }
 
+    @Test
+    public void hyperlinkFieldMacroRenderedTitleNotTruncated(@Mocked ModelFormField.HyperlinkField hyperlinkField) throws IOException {
+        final String description = "DESCRIPTION";
+        final String title = "TITLE";
+
+        new Expectations() {
+            {
+                hyperlinkField.getDescription(withNotNull()); result = description;
+                hyperlinkField.getTarget(withNotNull()); result = "#";
+                request.getAttribute("title"); result = title;
+            }
+        };
+
+        macroFormRenderer.renderHyperlinkField(appendable, new HashMap<>(), hyperlinkField);
+        assertAndGetMacroString("makeHyperlinkString", ImmutableMap.of("description", description, "title", title));
+    }
+
+    @Test
+    public void hyperlinkFieldMacroRenderedTruncatedNoTitle(@Mocked ModelFormField.HyperlinkField hyperlinkField) throws IOException {
+        final String description = "DESCRIPTION";
+
+        new Expectations() {
+            {
+                hyperlinkField.getDescription(withNotNull()); result = description;
+                hyperlinkField.getTarget(withNotNull()); result = "#";
+                request.getAttribute("descriptionSize"); result = 5;
+            }
+        };
+
+        macroFormRenderer.renderHyperlinkField(appendable, new HashMap<>(), hyperlinkField);
+        assertAndGetMacroString("makeHyperlinkString", ImmutableMap.of("description", "DESCR…", "title", description));
+    }
+
+    @Test
+    public void hyperlinkFieldMacroRenderedTruncatedWithTitle(@Mocked ModelFormField.HyperlinkField hyperlinkField) throws IOException {
+        final String description = "DESCRIPTION";
+
+        new Expectations() {
+            {
+                hyperlinkField.getDescription(withNotNull()); result = description;
+                hyperlinkField.getTarget(withNotNull()); result = "#";
+                request.getAttribute("descriptionSize"); result = 5;
+            }
+        };
+
+        macroFormRenderer.renderHyperlinkField(appendable, new HashMap<>(), hyperlinkField);
+        assertAndGetMacroString("makeHyperlinkString", ImmutableMap.of("description", "DESCR…", "title", description));

Review Comment:
   This test's name suggests proving that '#' is rendered for the title attribute, but instead it is proving that 'DESCRIPTION' is rendered. Was that the intention?



##########
framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/MacroFormRenderer.java:
##########
@@ -3005,6 +3006,9 @@ public void makeHyperlinkString(Appendable writer, String linkStyle, String targ
             if (UtilValidate.isNotEmpty(request.getAttribute("id"))) {
                 id = request.getAttribute("id").toString();
             }
+            if (UtilValidate.isNotEmpty(request.getAttribute("title"))) {
+                imgTitle = request.getAttribute("title").toString();

Review Comment:
   The logic sounds good, and even better is that you have asserted it with tests.
   Sorry to add to your work, but could you see if you can create the corresponding tests in RenderableFtlFormElementsBuilderTest.



-- 
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: notifications-unsubscribe@ofbiz.apache.org

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