[rfc][icedtea-web] make download indicator more compact
Adam Domurad
adomurad at redhat.com
Fri Jan 4 08:35:17 PST 2013
On 01/03/2013 11:07 AM, Jiri Vanek wrote:
> Hi!
>
> This patch is adding "show details" to download indicator. For One jar
> jnlp files it behaviour is unaffected. For Multiple jars there is just
> one progress bar, but can be shown "old stile" detailed one via
> clicking to "Show details". Then can be minimalised by "hide
> details". Button is localised.
>
> The button is nasty not nice.. Any better idea? but still this is
> better then previous approach.
>
> Looking forward to have this inside!
>
> Best rigards
> J.
Thanks for looking into this! I was actually comparing our download
indicator to proprietary just yesterday, while staring at multiple jars
downloading. The average user does not want to know about the jars being
downloaded, in fact I think such information can look scary :-).
> diff -r 9549226afa8f
> netx/net/sourceforge/jnlp/cache/DefaultDownloadIndicator.java
> --- a/netx/net/sourceforge/jnlp/cache/DefaultDownloadIndicator.java
> Thu Jan 03 09:54:16 2013 +0100
> +++ b/netx/net/sourceforge/jnlp/cache/DefaultDownloadIndicator.java
> Thu Jan 03 17:07:28 2013 +0100
> @@ -62,6 +62,7 @@
>
> /** shared constraint */
> static GridBagConstraints vertical;
> + static GridBagConstraints verticalNoClean;
> static GridBagConstraints verticalIndent;
> static {
> vertical = new GridBagConstraints();
> @@ -70,8 +71,12 @@
> vertical.fill = GridBagConstraints.HORIZONTAL;
> vertical.anchor = GridBagConstraints.WEST;
>
> + verticalNoClean = new GridBagConstraints();
> + verticalNoClean.weightx = 1.0;
> +
> verticalIndent = (GridBagConstraints) vertical.clone();
> verticalIndent.insets = new Insets(0, 10, 3, 0);
> +
Nit: These two blank lines (and all the ones added below) have tabs :-)
> }
>
> /**
> @@ -114,15 +119,15 @@
>
> frame.getContentPane().add(result, vertical);
> frame.pack();
> -
> if (!frame.isVisible()) {
> - Dimension screenSize =
> Toolkit.getDefaultToolkit().getScreenSize();
> - Insets insets =
> Toolkit.getDefaultToolkit().getScreenInsets(frame.getGraphicsConfiguration());
> - Dimension screen = new Dimension(screenSize.width -
> insets.left,
> - screenSize.height - insets.top);
> - frame.setLocation(screen.width - frame.getWidth(),
> - screen.height - frame.getHeight());
> + placeFrameToLowerRight();
> }
> + result.addComponentListener(new ComponentAdapter() {
> + @Override
> + public void componentResized(ComponentEvent e) {
> + placeFrameToLowerRight();
> + }
> + });
>
> frame.setVisible(true);
>
> @@ -131,6 +136,32 @@
> }
>
> /**
> + * The insets are calculated differently during first appearance
> + * and during another appearance in case of some configurations.
> + *
> + * So the first value is stored to avoid jumping of window during
> later packing
> + * of frame.
> + *
> + * However the second calculation is more correct:(
> + *
> + * This is affecting only multiple monitors which have different
> assets.
> + * The underlying issue is that
> Toolkit.getDefaultToolkit().getScreenInsets
> + * is returning assets for "random" monitor.
> + */
> + Dimension screen = null;
> +
> + private void placeFrameToLowerRight() throws HeadlessException {
> + if (screen == null) {
> + Dimension screenSize =
> Toolkit.getDefaultToolkit().getScreenSize();
> + Insets insets =
> Toolkit.getDefaultToolkit().getScreenInsets(frame.getGraphicsConfiguration());
> + screen = new Dimension(screenSize.width - insets.left,
> + screenSize.height - insets.top);
> + }
> + frame.setLocation(screen.width - frame.getWidth(),
> + screen.height - frame.getHeight());
> + }
> +
> + /**
> * Remove a download service listener that was obtained by
> * calling the getDownloadListener method from the shared
> * download info window.
> @@ -163,12 +194,26 @@
> * Groups the url progress in a panel.
> */
> static class DownloadPanel extends JPanel implements
> DownloadServiceListener {
> -
> + private static enum States{
> + oneJAr, collapsed, detailed;
Enums should be all-caps
> +
> + }
> +
> + private static final String DETAILS=R("ButShowDetails");
> + private static final String HIDE_DETAILS=R("ButHideDetails");
> /** the download name */
> private String downloadName;
>
> /** Downloading part: */
> private JLabel header = new JLabel();
> + /** Show/hide details button: */
> + private JButton details = new JButton(DETAILS);
> + /** used instead of details button in case of one jar*/
> + private JLabel delimiter = new JLabel("");
> + /** all already created progress bars*/
> + private List<ProgressPanel> progressPanels = new
> ArrayList<ProgressPanel>();
> + private States state=States.oneJAr;
> + private ProgressPanel mainProgressPanel;
>
> /** list of URLs being downloaded */
> private List<URL> urls = new ArrayList<URL>();
> @@ -176,6 +221,7 @@
> /** list of ProgressPanels */
> private List<ProgressPanel> panels = new
> ArrayList<ProgressPanel>();
>
> +
Nit: two blank lines are unnecessary.
> /**
> * Create a new download panel for with the specified download
> * name.
> @@ -184,9 +230,35 @@
> setLayout(new GridBagLayout());
>
> this.downloadName = downloadName;
> - this.add(header, vertical);
> + this.add(header, verticalNoClean);
> header.setFont(header.getFont().deriveFont(Font.BOLD));
> -
> + this.add(delimiter, vertical);
> + details.addActionListener(new ActionListener() {
> + @Override
> + public void actionPerformed(ActionEvent e) {
> + if (state == States.detailed) {
> + state = States.collapsed;
> + details.setText(DETAILS);
> + for (ProgressPanel progressPanel :
> progressPanels) {
> + remove(progressPanel);
> + }
> + add(mainProgressPanel, verticalIndent);
> + synchronized (frameMutex) {
> + frame.pack();
> + }
> + } else {
> + state = States.detailed;
> + details.setText(HIDE_DETAILS);
> + remove(mainProgressPanel);
> + for (ProgressPanel progressPanel :
> progressPanels) {
> + add(progressPanel, verticalIndent);
> + }
> + synchronized (frameMutex) {
> + frame.pack();
> + }
> + }
> + }
> + });
> setOverallPercent(0);
> }
>
> @@ -196,14 +268,25 @@
> protected void addProgressPanel(URL url, String version) {
> if (!urls.contains(url)) {
> ProgressPanel panel = new ProgressPanel(url, version);
> -
> - add(panel, verticalIndent);
> + if (state != States.collapsed) {
> + add(panel, verticalIndent);
> + }
> + progressPanels.add(panel);
> + urls.add(url);
> + panels.add(panel);
> + if (panels.size() == 2){
Small comment here would be good explaining why this 'magic number'
> + remove(panels.get(0));
> + remove(panels.get(1));
> + remove(delimiter);
> + add(details,vertical);
> + mainProgressPanel=new ProgressPanel();
> + add(mainProgressPanel, verticalIndent);
> + state=States.collapsed;
> + }
> synchronized (frameMutex) {
> frame.pack();
> }
>
> - urls.add(url);
> - panels.add(panel);
> }
> }
>
> @@ -219,10 +302,10 @@
> addProgressPanel(url, version);
>
> setOverallPercent(overallPercent);
> -
> ProgressPanel panel = panels.get(urls.indexOf(url));
> panel.setProgress(readSoFar, total);
> panel.repaint();
> +
> }
> };
> SwingUtilities.invokeLater(r);
> @@ -230,12 +313,28 @@
>
> /**
> * Sets the overall percent completed.
> + * should be called via invokeLater
> */
> public void setOverallPercent(int percent) {
> // don't get whole string from resource and sub in
> // values because it'll be doing a MessageFormat for
> // each update.
> header.setText(downloading + " " + downloadName + ": " +
> percent + "% " + complete + ".");
> + Container c = header.getParent();
> + //we need to adapt nbboth panels and also frame to new
> length of header text
nbboth > both
> + while (c != null) {
> + c.invalidate();
> + c.validate();
> + if (c instanceof Window){
> + ((Window) c).pack();
> + }
> + c=c.getParent();
> + }
> +
> + if (mainProgressPanel != null) {
> + mainProgressPanel.setProgress(percent, 100);
> + mainProgressPanel.repaint();
> + }
> }
>
> /**
> @@ -276,12 +375,28 @@
>
> private long total;
> private long readSoFar;
> + private Dimension size = new Dimension(80, 15);
>
> + ProgressPanel() {
> + bar.setMinimumSize(size);
> + bar.setPreferredSize(size);
> + bar.setOpaque(false);
> +
> + setLayout(new GridBagLayout());
> +
> + GridBagConstraints gbc = new GridBagConstraints();
> + styleGridBagConstraints(gbc);
> + add(bar, gbc);
> + }
> +
> ProgressPanel(URL url, String version) {
> - JLabel location = new JLabel(" " + url.getHost() + "/" +
> url.getFile());
> + this(" " + url.getHost() + "/" + url.getFile(),version);
> + }
> + ProgressPanel(String caption, String version) {
> + JLabel location = new JLabel(caption);
>
> - bar.setMinimumSize(new Dimension(80, 15));
> - bar.setPreferredSize(new Dimension(80, 15));
> + bar.setMinimumSize(size);
> + bar.setPreferredSize(size);
> bar.setOpaque(false);
>
> setLayout(new GridBagLayout());
> @@ -291,12 +406,8 @@
> gbc.fill = GridBagConstraints.NONE;
> gbc.gridwidth = GridBagConstraints.RELATIVE;
> add(bar, gbc);
> -
> - gbc.insets = new Insets(0, 3, 0, 0);
> - gbc.weightx = 1.0;
> - gbc.fill = GridBagConstraints.HORIZONTAL;
> - gbc.gridwidth = GridBagConstraints.REMAINDER;
> - gbc.anchor = GridBagConstraints.WEST;
> +
> + styleGridBagConstraints(gbc);
> add(location, gbc);
> }
>
> @@ -325,6 +436,14 @@
> g.fillRect(x + 1, y + 1, divide - 1, h - 1);
> }
> }
> +
> + private void styleGridBagConstraints(GridBagConstraints gbc) {
> + gbc.insets = new Insets(0, 3, 0, 0);
> + gbc.weightx = 1.0;
> + gbc.fill = GridBagConstraints.HORIZONTAL;
> + gbc.gridwidth = GridBagConstraints.REMAINDER;
> + gbc.anchor = GridBagConstraints.WEST;
> + }
> };
>
> }
Code looks OK nits aside.
As for button ... yes it is nasty. IMO maybe we can take different
approach here. The people interested in how the individual jars are
downloading would mainly be 1. us trying to fix jar loading issues, 2.
developers looking into jar loading issues.
So what if the ICEDTEAPLUGIN_DEBUG environment variable turned this
on/off ? It can be argued it is a bit hard for developers to access, but
it isn't any easier to access this information from oracle plugin.
As well, I would be very happy to see the loading bar integrated with
the splash screen (if splash screen is on). I believe this would nicely
complement the user friendliness of the splash screen.
Happy hacking,
-Adam
More information about the distro-pkg-dev
mailing list