From: Henri Sara <hesara@vaadin.com>
Date: Mon, 9 Nov 2015 12:52:46 +0000 (+0200)
Subject: Refactor ComboBox item painting (#19929)
X-Git-Tag: 8.0.0.alpha1~228
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=e58fe62230285a4b5b7892ca911770e43db2eee3;p=vaadin-framework.git

Refactor ComboBox item painting (#19929)

This is an intermediate step towards a different approach
to sending items from server to client.

An unused null selection item attribute was removed, as the
information is currently communicated outside the item.

Change-Id: I7b2cd34d2d05287f91d6859fca28337dccdbe8df
---

diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java
index 2e58befaa5..bba261a110 100644
--- a/server/src/main/java/com/vaadin/ui/ComboBox.java
+++ b/server/src/main/java/com/vaadin/ui/ComboBox.java
@@ -230,6 +230,31 @@ public class ComboBox extends AbstractSelect implements
                 && getFilteringMode() != FilteringMode.OFF;
     }
 
+    /**
+     * A class representing an item in a ComboBox for server to client
+     * communication. This class is for internal use only and subject to change.
+     * 
+     * @since
+     */
+    private static class ComboBoxItem implements Serializable {
+        String key = "";
+        String caption = "";
+        String style = null;
+        Resource icon = null;
+
+        // constructor for a null item
+        public ComboBoxItem() {
+        }
+
+        public ComboBoxItem(String key, String caption, String style,
+                Resource icon) {
+            this.key = key;
+            this.caption = caption;
+            this.style = style;
+            this.icon = icon;
+        }
+    }
+
     @Override
     public void paintContent(PaintTarget target) throws PaintException {
         isPainting = true;
@@ -268,8 +293,6 @@ public class ComboBox extends AbstractSelect implements
             // Paints the options and create array of selected id keys
             int keyIndex = 0;
 
-            target.startTag("options");
-
             if (currentPage < 0) {
                 optionRequest = false;
                 currentPage = 0;
@@ -295,14 +318,12 @@ public class ComboBox extends AbstractSelect implements
             final boolean paintNullSelection = needNullSelectOption
                     && currentPage == 0 && !nullFilteredOut;
 
-            if (paintNullSelection) {
-                target.startTag("so");
-                target.addAttribute("caption", "");
-                target.addAttribute("key", "");
-
-                paintItemStyle(target, null);
+            List<ComboBoxItem> items = new ArrayList<ComboBoxItem>();
 
-                target.endTag("so");
+            if (paintNullSelection) {
+                ComboBoxItem item = new ComboBoxItem();
+                item.style = getItemStyle(null);
+                items.add(item);
             }
 
             final Iterator<?> i = options.iterator();
@@ -322,24 +343,31 @@ public class ComboBox extends AbstractSelect implements
                 final String key = itemIdMapper.key(id);
                 final String caption = getItemCaption(id);
                 final Resource icon = getItemIcon(id);
+
                 getCaptionChangeListener().addNotifierForItem(id);
 
-                // Paints the option
-                target.startTag("so");
-                if (icon != null) {
-                    target.addAttribute("icon", icon);
-                }
-                target.addAttribute("caption", caption);
-                if (id != null && id.equals(getNullSelectionItemId())) {
-                    target.addAttribute("nullselection", true);
-                }
-                target.addAttribute("key", key);
+                // Prepare to paint the option
+                ComboBoxItem item = new ComboBoxItem(key, caption,
+                        getItemStyle(id), icon);
+                items.add(item);
                 if (keyIndex < selectedKeys.length && isSelected(id)) {
                     // at most one item can be selected at a time
                     selectedKeys[keyIndex++] = key;
                 }
+            }
 
-                paintItemStyle(target, id);
+            // paint the items
+            target.startTag("options");
+            for (ComboBoxItem item : items) {
+                target.startTag("so");
+                if (item.icon != null) {
+                    target.addAttribute("icon", item.icon);
+                }
+                target.addAttribute("caption", item.caption);
+                target.addAttribute("key", item.key);
+                if (item.style != null) {
+                    target.addAttribute("style", item.style);
+                }
 
                 target.endTag("so");
             }
@@ -377,14 +405,11 @@ public class ComboBox extends AbstractSelect implements
 
     }
 
-    private void paintItemStyle(PaintTarget target, Object itemId)
-            throws PaintException {
+    private String getItemStyle(Object itemId) throws PaintException {
         if (itemStyleGenerator != null) {
-            String style = itemStyleGenerator.getStyle(this, itemId);
-            if (style != null && !style.isEmpty()) {
-                target.addAttribute("style", style);
-            }
+            return itemStyleGenerator.getStyle(this, itemId);
         }
+        return null;
     }
 
     /**