From 110b500847f1fd8dc5b8cc6ac83f7dbebcccdb8c Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 3 Aug 2017 19:31:50 +0100 Subject: [PATCH] gapic: Address review comments. --- .../src/main/com/google/gapid/MainWindow.java | 33 +++++-------------- .../com/google/gapid/models/Settings.java | 2 +- .../com/google/gapid/util/UpdateWatcher.java | 8 ++--- .../com/google/gapid/views/StatusBar.java | 30 +++++------------ .../main/com/google/gapid/widgets/Theme.java | 3 -- 5 files changed, 22 insertions(+), 54 deletions(-) diff --git a/gapic/src/main/com/google/gapid/MainWindow.java b/gapic/src/main/com/google/gapid/MainWindow.java index a44fb25c05..7b03355ced 100644 --- a/gapic/src/main/com/google/gapid/MainWindow.java +++ b/gapic/src/main/com/google/gapid/MainWindow.java @@ -24,6 +24,7 @@ import static com.google.gapid.widgets.GotoAtom.showGotoAtomDialog; import static com.google.gapid.widgets.GotoMemory.showGotoMemoryDialog; import static com.google.gapid.widgets.Licenses.showLicensesDialog; +import static com.google.gapid.widgets.Widgets.scheduleIfNotDisposed; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -60,12 +61,6 @@ import com.google.gapid.widgets.TabArea.TabInfo; import com.google.gapid.widgets.Widgets; -import java.awt.Desktop; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.logging.Level; -import java.util.logging.Logger; import org.eclipse.jface.action.Action; import org.eclipse.jface.action.IAction; import org.eclipse.jface.action.MenuManager; @@ -76,6 +71,7 @@ import org.eclipse.swt.graphics.Rectangle; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; +import org.eclipse.swt.program.Program; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control; import org.eclipse.swt.widgets.Shell; @@ -92,7 +88,6 @@ * The main {@link ApplicationWindow} containing all of the UI components. */ public class MainWindow extends ApplicationWindow { - private static final Logger LOG = Logger.getLogger(MainWindow.class.getName()); protected final Client client; protected final ModelsAndWidgets maw; protected Action gotoAtom, gotoMemory; @@ -233,7 +228,7 @@ protected Control createContents(Composite parent) { SashForm splitter = new SashForm(shell, SWT.VERTICAL); splitter.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, true)); - statusBar = new StatusBar(shell, widgets().theme); + statusBar = new StatusBar(shell); statusBar.setLayoutData(new GridData(SWT.FILL, SWT.BOTTOM, true, false)); scrubber = new ThumbnailScrubber(splitter, models(), widgets()); @@ -281,23 +276,13 @@ public void onStateFollowed(Path.Any path) { } private void watchForUpdates() { - Desktop desktop = Desktop.getDesktop(); - if (desktop.isSupported(Desktop.Action.BROWSE)) { - new UpdateWatcher(maw.models().settings, client, (release) -> { - try { - URI uri = new URI(release.getBrowserUrl()); - statusBar.setNotification("New update available", () -> { - try { - desktop.browse(uri); - } catch (IOException e) { - LOG.log(Level.WARNING, "Couldn't launch browser", e); - } - }); - } catch (URISyntaxException e) { - LOG.log(Level.WARNING, "Invalid release URI", e); - }; + new UpdateWatcher(maw.models().settings, client, (release) -> { + scheduleIfNotDisposed(statusBar, () -> { + statusBar.setNotification("New update available", () -> { + Program.launch(release.getBrowserUrl()); + }); }); - } + }); } @Override diff --git a/gapic/src/main/com/google/gapid/models/Settings.java b/gapic/src/main/com/google/gapid/models/Settings.java index c44cafcbc6..8b46ef7f6e 100644 --- a/gapic/src/main/com/google/gapid/models/Settings.java +++ b/gapic/src/main/com/google/gapid/models/Settings.java @@ -76,7 +76,7 @@ public class Settings { public String adb = ""; public boolean autoCheckForUpdates = true; public boolean updateAvailable = false; - public long lastCheckForUpdates = 0; // milliseconds since January 1, 1970, 00:00:00 GMT. + public long lastCheckForUpdates = 0; // milliseconds since midnight, January 1, 1970 UTC. public static Settings load() { Settings result = new Settings(); diff --git a/gapic/src/main/com/google/gapid/util/UpdateWatcher.java b/gapic/src/main/com/google/gapid/util/UpdateWatcher.java index 8e1c77e63b..1381194b3c 100644 --- a/gapic/src/main/com/google/gapid/util/UpdateWatcher.java +++ b/gapic/src/main/com/google/gapid/util/UpdateWatcher.java @@ -19,7 +19,6 @@ import com.google.gapid.models.Settings; import com.google.gapid.proto.service.Service.Release; import com.google.gapid.server.Client; -import java.util.Date; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -27,8 +26,7 @@ * Utility class for checking for new releases of GAPID. */ public class UpdateWatcher { - private static final int CHECK_INTERVAL_HOURS = 8; - private static final long CHECK_INTERVAL_MS = CHECK_INTERVAL_HOURS * 60 * 60 * 1000; + private static final long CHECK_INTERVAL_MS = TimeUnit.HOURS.toMillis(8); private static final boolean INCLUDE_PRE_RELEASES = true; private final Settings settings; @@ -53,7 +51,7 @@ public UpdateWatcher(Settings settings, Client client, Listener listener) { } private void scheduleCheck() { - long now = new Date().getTime(); + long now = System.currentTimeMillis(); long timeSinceLastUpdateMS = now - settings.lastCheckForUpdates; long delay = Math.max(CHECK_INTERVAL_MS - timeSinceLastUpdateMS, 0); Scheduler.EXECUTOR.schedule(this::doCheck, delay, TimeUnit.MILLISECONDS); @@ -73,7 +71,7 @@ private void doCheck() { /* never mind */ } } - settings.lastCheckForUpdates = new Date().getTime(); + settings.lastCheckForUpdates = System.currentTimeMillis(); settings.save(); scheduleCheck(); } diff --git a/gapic/src/main/com/google/gapid/views/StatusBar.java b/gapic/src/main/com/google/gapid/views/StatusBar.java index a63fd7fb2d..4642904568 100644 --- a/gapic/src/main/com/google/gapid/views/StatusBar.java +++ b/gapic/src/main/com/google/gapid/views/StatusBar.java @@ -15,59 +15,47 @@ */ package com.google.gapid.views; -import static com.google.gapid.widgets.Widgets.scheduleIfNotDisposed; - -import com.google.gapid.widgets.Theme; import com.google.gapid.widgets.Widgets; import org.eclipse.swt.SWT; import org.eclipse.swt.layout.GridData; import org.eclipse.swt.layout.GridLayout; import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Label; +import org.eclipse.swt.widgets.Link; /** * Displays status information at the bottom of the main window. */ public class StatusBar extends Composite { - private final Theme theme; private final Label status; - private final Label notification; + private final Link notification; private Runnable onNotificationClick; - public StatusBar(Composite parent, Theme theme) { + public StatusBar(Composite parent) { super(parent, SWT.NONE); - this.theme = theme; - setLayout(new GridLayout(2, false)); status = Widgets.createLabel(this, ""); status.setLayoutData(new GridData(SWT.LEFT, SWT.FILL, true, false)); - notification = Widgets.createLabel(this, ""); - notification.setLayoutData(new GridData(SWT.RIGHT, SWT.FILL, false, false)); - notification.addListener(SWT.MouseDown, (e) -> { + notification = Widgets.createLink(this, "", (e) -> { if (onNotificationClick != null) { onNotificationClick.run(); } }); + notification.setLayoutData(new GridData(SWT.RIGHT, SWT.FILL, false, false)); } /** - * Updates the notification to the given text. Can be safely called on a non-UI thread. + * Updates the notification to the given text. * * @param text the notification text. * @param onClick the optional notifiction click handler. */ public void setNotification(String text, Runnable onClick) { - scheduleIfNotDisposed(this, () -> { - notification.setText(text); - notification.setForeground(onClick != null ? - theme.linkForeground() : theme.notificationForeground()); - - onNotificationClick = onClick; - - layout(); - }); + notification.setText((onClick != null) ? "" + text + "" : text); + onNotificationClick = onClick; + layout(); } } diff --git a/gapic/src/main/com/google/gapid/widgets/Theme.java b/gapic/src/main/com/google/gapid/widgets/Theme.java index 92cde39ba0..e5e7d0f9bd 100644 --- a/gapic/src/main/com/google/gapid/widgets/Theme.java +++ b/gapic/src/main/com/google/gapid/widgets/Theme.java @@ -131,9 +131,6 @@ public interface Theme { @RGB(argb = 0xff000000) public Color imageCursorDark(); @RGB(argb = 0xffffffff) public Color imageCursorLight(); - @RGB(argb = 0xff0000ee) public Color linkForeground(); - @RGB(argb = 0xff000000) public Color notificationForeground(); - @TextStyle(foreground = 0xa9a9a9) public Styler structureStyler(); @TextStyle(foreground = 0x0000ee) public Styler identifierStyler(); @TextStyle(bold = true) public Styler labelStyler();