Skip to content

Commit c87a375

Browse files
authored
Merge pull request #4780 from gchq/gh-4778
#4778 Improve menu text rendering
2 parents 2a34d8c + 2aaa69e commit c87a375

File tree

18 files changed

+117
-75
lines changed

18 files changed

+117
-75
lines changed

Diff for: stroom-core-client-widget/src/main/java/stroom/cell/info/client/HoverActionMenuCell.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@ public void render(final Context context, final T value, final SafeHtmlBuilder s
159159
ICON_CLASS_NAME,
160160
HOVER_ACTION_MENU_CLASS_NAME);
161161

162-
if (menuItems.size() == 1
163-
&& menuItems.get(0) instanceof MenuItem
164-
&& ((MenuItem) menuItems.get(0)).getCommand() != null) {
162+
if (menuItems.size() == 1 &&
163+
menuItems.get(0) instanceof final MenuItem menuItem &&
164+
menuItem.getCommand() != null) {
165165
// Single item with a command so no menu popup needed
166-
final String menuItemText = ((MenuItem) menuItems.get(0)).getText();
167-
sb.append(template.divWithToolTip(menuItemText, icon));
166+
sb.append(template.divWithToolTip(GwtNullSafe
167+
.getOrElse(menuItem.getText(), SafeHtml::asString, ""), icon));
168168
} else {
169169
// Build the menu popup
170170
sb.append(template.divWithToolTip("Actions...", icon));

Diff for: stroom-core-client-widget/src/main/java/stroom/editor/client/presenter/Action.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package stroom.editor.client.presenter;
1818

19+
import com.google.gwt.safehtml.shared.SafeHtml;
20+
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
21+
1922
public class Action {
2023

2124
private final Runnable executeHandler;
@@ -56,7 +59,7 @@ public void setToDefaultAvailability() {
5659
setAvailable(defaultAvailability);
5760
}
5861

59-
public String getText() {
60-
return text;
62+
public SafeHtml getText() {
63+
return SafeHtmlUtils.fromString(text);
6164
}
6265
}

Diff for: stroom-core-client-widget/src/main/java/stroom/editor/client/presenter/Option.java

+12-3
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616

1717
package stroom.editor.client.presenter;
1818

19+
import com.google.gwt.safehtml.shared.SafeHtml;
20+
import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
21+
1922
public class Option {
2023

2124
private ChangeHandler changeHandler;
@@ -94,11 +97,17 @@ public void setChangeHandler(final ChangeHandler changeHandler) {
9497
this.changeHandler = changeHandler;
9598
}
9699

97-
public String getText() {
100+
public SafeHtml getText() {
98101
if (on) {
99-
return text + " (<span class=\"editor-menu-option-on\">ON</span>)";
102+
return new SafeHtmlBuilder()
103+
.appendEscaped(text)
104+
.appendHtmlConstant(" (<span class=\"editor-menu-option-on\">ON</span>)")
105+
.toSafeHtml();
100106
} else {
101-
return text + " (<span class=\"editor-menu-option-off\">OFF</span>)";
107+
return new SafeHtmlBuilder()
108+
.appendEscaped(text)
109+
.appendHtmlConstant(" (<span class=\"editor-menu-option-off\">OFF</span>)")
110+
.toSafeHtml();
102111
}
103112
}
104113

Diff for: stroom-core-client-widget/src/main/java/stroom/editor/client/view/EditorMenuPresenter.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import stroom.widget.menu.client.presenter.ShowMenuEvent;
2525
import stroom.widget.popup.client.presenter.PopupPosition;
2626

27+
import com.google.gwt.safehtml.shared.SafeHtml;
2728
import com.google.gwt.user.client.Command;
2829

2930
import java.util.ArrayList;
@@ -83,7 +84,7 @@ private void addMenuItem(int position, final List<Item> menuItems, final Action
8384
}
8485
}
8586

86-
private Item createItem(final String text, final Command command, final int position) {
87+
private Item createItem(final SafeHtml text, final Command command, final int position) {
8788
return new IconMenuItem.Builder()
8889
.priority(position)
8990
.text(text)

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/IconMenuItem.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import stroom.util.shared.GwtNullSafe;
2323
import stroom.widget.util.client.KeyBinding.Action;
2424

25+
import com.google.gwt.safehtml.shared.SafeHtml;
2526
import com.google.gwt.user.client.Command;
2627

2728
public class IconMenuItem extends MenuItem {
@@ -35,7 +36,7 @@ protected IconMenuItem(final int priority,
3536
final SvgImage enabledIcon,
3637
final SvgImage disabledIcon,
3738
final IconColour iconColour,
38-
final String text,
39+
final SafeHtml text,
3940
final Action action,
4041
final boolean enabled,
4142
final Command command,

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/IconParentMenuItem.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import stroom.widget.util.client.FutureImpl;
2323
import stroom.widget.util.client.KeyBinding.Action;
2424

25+
import com.google.gwt.safehtml.shared.SafeHtml;
26+
2527
import java.util.List;
2628

2729
public class IconParentMenuItem extends IconMenuItem implements HasChildren {
@@ -32,7 +34,7 @@ protected IconParentMenuItem(final int priority,
3234
final SvgImage enabledIcon,
3335
final SvgImage disabledIcon,
3436
final IconColour iconColour,
35-
final String text,
37+
final SafeHtml text,
3638
final Action action,
3739
final boolean enabled,
3840
final boolean highlight,

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/InfoMenuItem.java

+13-12
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,23 @@
33
import stroom.widget.util.client.KeyBinding.Action;
44

55
import com.google.gwt.safehtml.shared.SafeHtml;
6+
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
67
import com.google.gwt.user.client.Command;
78

89
public class InfoMenuItem extends MenuItem {
910

10-
private final SafeHtml safeHtml;
11+
private final SafeHtml text;
1112

12-
public InfoMenuItem(final SafeHtml safeHtml,
13+
public InfoMenuItem(final SafeHtml text,
1314
final Action action,
1415
final Boolean enabled,
1516
final Command command) {
16-
super(0, "", action, enabled, command);
17-
this.safeHtml = safeHtml;
17+
super(0, SafeHtmlUtils.EMPTY_SAFE_HTML, action, enabled, command);
18+
this.text = text;
1819
}
1920

20-
public SafeHtml getSafeHtml() {
21-
return safeHtml;
21+
public SafeHtml getText() {
22+
return text;
2223
}
2324

2425
public static Builder builder() {
@@ -27,16 +28,16 @@ public static Builder builder() {
2728

2829
public static class Builder {
2930

30-
private SafeHtml safeHtml = null;
31+
private SafeHtml text = SafeHtmlUtils.EMPTY_SAFE_HTML;
3132
private Action action = null;
3233
private Command command = null;
3334
private boolean enabled = true;
3435

3536
Builder() {
3637
}
3738

38-
public Builder withSafeHtml(final SafeHtml safeHtml) {
39-
this.safeHtml = safeHtml;
39+
public Builder text(final SafeHtml text) {
40+
this.text = text;
4041
return this;
4142
}
4243

@@ -45,12 +46,12 @@ public Builder action(final Action action) {
4546
return this;
4647
}
4748

48-
public Builder withCommand(final Command command) {
49+
public Builder command(final Command command) {
4950
this.command = command;
5051
return this;
5152
}
5253

53-
public Builder withEnabledState(final boolean enabled) {
54+
public Builder enabled(final boolean enabled) {
5455
this.enabled = enabled;
5556
return this;
5657
}
@@ -62,7 +63,7 @@ public Builder disabled() {
6263

6364
public Item build() {
6465
return new InfoMenuItem(
65-
safeHtml,
66+
text,
6667
action,
6768
enabled,
6869
command);

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/KeyedParentMenuItem.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import stroom.widget.util.client.FutureImpl;
2323
import stroom.widget.util.client.KeyBinding.Action;
2424

25+
import com.google.gwt.safehtml.shared.SafeHtml;
26+
2527
import java.util.List;
2628
import java.util.Objects;
2729

@@ -34,7 +36,7 @@ public class KeyedParentMenuItem extends IconMenuItem implements HasChildren {
3436
final SvgImage enabledIcon,
3537
final SvgImage disabledIcon,
3638
final IconColour iconColour,
37-
final String text,
39+
final SafeHtml text,
3840
final Action action,
3941
final boolean enabled,
4042
final MenuItems menuItems,

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/MenuItem.java

+12-5
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,20 @@
1717
package stroom.widget.menu.client.presenter;
1818

1919
import stroom.widget.util.client.KeyBinding.Action;
20+
import stroom.widget.util.client.SafeHtmlUtil;
2021

22+
import com.google.gwt.safehtml.shared.SafeHtml;
2123
import com.google.gwt.user.client.Command;
2224

2325
public abstract class MenuItem extends Item {
2426

25-
private final String text;
27+
private final SafeHtml text;
2628
private final Action action;
2729
private final Command command;
2830
private final boolean enabled;
2931

3032
protected MenuItem(final int priority,
31-
final String text,
33+
final SafeHtml text,
3234
final Action action,
3335
final boolean enabled,
3436
final Command command) {
@@ -39,7 +41,7 @@ protected MenuItem(final int priority,
3941
this.command = command;
4042
}
4143

42-
public String getText() {
44+
public SafeHtml getText() {
4345
return text;
4446
}
4547

@@ -62,16 +64,21 @@ public Command getCommand() {
6264
protected abstract static class AbstractBuilder<T extends MenuItem, B extends MenuItem.AbstractBuilder<T, ?>>
6365
extends Item.AbstractBuilder<T, B> {
6466

65-
protected String text;
67+
protected SafeHtml text;
6668
protected Action action;
6769
protected Command command;
6870
protected boolean enabled = true;
6971

70-
public B text(final String text) {
72+
public B text(final SafeHtml text) {
7173
this.text = text;
7274
return self();
7375
}
7476

77+
public B text(final String text) {
78+
this.text = SafeHtmlUtil.from(text);
79+
return self();
80+
}
81+
7582
public B action(final Action action) {
7683
this.action = action;
7784
return self();

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/MenuItemCell.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public static class MenuItemAppearance implements Appearance<MenuItem> {
125125
public void render(final MenuItemCell cell, final Context context, final MenuItem value,
126126
final SafeHtmlBuilder sb) {
127127
if (value.getText() != null) {
128-
sb.append(SafeHtmlUtils.fromTrustedString(value.getText()));
128+
sb.append(value.getText());
129129
}
130130
}
131131
}
@@ -172,7 +172,7 @@ public void render(final MenuItemCell cell,
172172

173173

174174
inner.append(
175-
TEMPLATE.inner("menuItem-text", SafeHtmlUtils.fromTrustedString(value.getText())));
175+
TEMPLATE.inner("menuItem-text", value.getText()));
176176

177177
if (value.getAction() != null) {
178178
final String shortcut = KeyBinding.getShortcut(value.getAction());
@@ -184,7 +184,7 @@ public void render(final MenuItemCell cell,
184184

185185
// If this is a parent menu item, render an arrow to the right-hand side
186186
if ((value instanceof IconParentMenuItem || value instanceof KeyedParentMenuItem)
187-
&& value.isEnabled()) {
187+
&& value.isEnabled()) {
188188
inner.append(SvgImageUtil.toSafeHtml(SvgImage.ARROW_RIGHT, "menuItem-expandArrow"));
189189
}
190190

@@ -235,7 +235,7 @@ public void render(final MenuItemCell cell,
235235
inner.append(
236236
TEMPLATE.inner(
237237
"menuItem-simpleText",
238-
SafeHtmlUtils.fromTrustedString(value.getText())));
238+
value.getText()));
239239

240240
if (value.getAction() != null) {
241241
final String shortcut = KeyBinding.getShortcut(value.getAction());
@@ -288,12 +288,12 @@ public void render(final MenuItemCell cell,
288288
final Context context,
289289
final InfoMenuItem value,
290290
final SafeHtmlBuilder sb) {
291-
if (value.getSafeHtml() != null) {
291+
if (value.getText() != null) {
292292
final SafeHtmlBuilder inner = new SafeHtmlBuilder();
293293

294294
inner.append(TEMPLATE.inner(
295295
"menuItem-simpleText",
296-
value.getSafeHtml()));
296+
value.getText()));
297297

298298
sb.append(TEMPLATE.outer(
299299
"menuItem-outer infoMenuItem",

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/MenuItems.java

+8-9
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.google.inject.Singleton;
2020

2121
import java.util.ArrayList;
22-
import java.util.Collections;
2322
import java.util.Comparator;
2423
import java.util.HashMap;
2524
import java.util.HashSet;
@@ -53,7 +52,7 @@ private void compress() {
5352
final Set<Item> items = entry.getValue();
5453
// Sort the items.
5554
final List<Item> sortedItems = new ArrayList<>(items);
56-
Collections.sort(sortedItems, new ItemComparator());
55+
sortedItems.sort(new ItemComparator());
5756

5857
final List<Item> compressedItems = new ArrayList<>();
5958
for (final Item item : sortedItems) {
@@ -63,15 +62,15 @@ private void compress() {
6362
}
6463

6564
// Cleanse the child items.
66-
if (compressedItems.size() > 0) {
65+
if (!compressedItems.isEmpty()) {
6766
// Remove leading separators.
68-
while (compressedItems.size() > 0 && compressedItems.get(0) instanceof Separator) {
67+
while (!compressedItems.isEmpty() && compressedItems.get(0) instanceof Separator) {
6968
compressedItems.remove(0);
7069
}
7170

7271
// Remove trailing separators.
73-
while (compressedItems.size() > 0
74-
&& compressedItems.get(compressedItems.size() - 1) instanceof Separator) {
72+
while (!compressedItems.isEmpty()
73+
&& compressedItems.get(compressedItems.size() - 1) instanceof Separator) {
7574
compressedItems.remove(compressedItems.size() - 1);
7675
}
7776

@@ -108,7 +107,7 @@ public void clear() {
108107
@Override
109108
public String toString() {
110109
final List<MenuKey> keys = new ArrayList<>(menuItems.keySet());
111-
Collections.sort(keys, new MenuKeyComparator());
110+
keys.sort(new MenuKeyComparator());
112111

113112
final StringBuilder sb = new StringBuilder();
114113
for (final MenuKey key : keys) {
@@ -119,7 +118,7 @@ public String toString() {
119118
final Set<Item> items = menuItems.get(key);
120119
// Sort the items.
121120
final List<Item> sortedItems = new ArrayList<>(items);
122-
Collections.sort(sortedItems, new ItemComparator());
121+
sortedItems.sort(new ItemComparator());
123122

124123
for (final Item item : sortedItems) {
125124
sb.append(" - ");
@@ -136,7 +135,7 @@ public static class ItemComparator implements Comparator<Item> {
136135
public int compare(final Item o1, final Item o2) {
137136
if (o1.getPriority() == o2.getPriority()) {
138137
if (o1 instanceof MenuItem && o2 instanceof MenuItem) {
139-
return ((MenuItem) o1).getText().compareTo(((MenuItem) o2).getText());
138+
return ((MenuItem) o1).getText().asString().compareTo(((MenuItem) o2).getText().asString());
140139
} else if (o1 instanceof Separator && o2 instanceof Separator) {
141140
return 0;
142141
} else if (o1 instanceof Separator) {

Diff for: stroom-core-client-widget/src/main/java/stroom/widget/menu/client/presenter/SimpleMenuItem.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818

1919
import stroom.widget.util.client.KeyBinding.Action;
2020

21+
import com.google.gwt.safehtml.shared.SafeHtml;
2122
import com.google.gwt.user.client.Command;
2223

2324
public class SimpleMenuItem extends MenuItem {
2425

2526
protected SimpleMenuItem(final int priority,
26-
final String text,
27+
final SafeHtml text,
2728
final Action action,
2829
final boolean enabled,
2930
final Command command) {

0 commit comments

Comments
 (0)