[rfc][icedtea-web] make download indicator more compact

[Jiri Vanek]

On 01/07/2013 08:00 PM, Adam Domurad wrote:
> On 01/07/2013 09:41 AM, Jiri Vanek wrote:
>> On 01/04/2013 05:35 PM, Adam Domurad wrote:
>>> 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.
>>
>> Thanx for nits. Should be fixed now
>>>
>>> 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.
>>
>> I think I don't like this idea :(
>
> No problem, but can you elaborate why not?

When it took to long to downld application, I'm happy to see whats going on. Maybe I can spot some 3rd party jar to be stuck and then I can ping them. And I definitely do not want to look for environmetn variables.
>
>> I have added an attempt to use an icon, And I must say it is much better. Btw, I have created this icon.. so no wonders here :) But looks quite cool :)
>
> It looks OK :-) The glass part could use some transparency.
blah :)

> There should be a slightly different icon from going from detailed ->compact.
As you wish, however, I do not like the red cross I have added :(

>
>>
>>>
>>> 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.
>>
>> You mean to sync progress on progress bar with the one in splash? Or also showing the downlaoded resources somehow and show details button in splash?
>
> I was thinking always have progress bar on splash, and have detailed view hide/show the download bars in corner. IMO this is good for now, except:
I agree. It can came in some future.

> 1. visual glitches such as http://i.imgur.com/Oveuh.png (with text flickering from middle to top every frame). As well on dual monitors expanding/compacting view constantly changed screens for me.
Well I was not able to reproduce today :-/
However I have changed the implementation and So I hope that it will behave better in your configuration.

Please note, now this patch is depending on [rfc][icedtea-web] centre of dialogues to centre of active monitor.
Sorry for inconvenience here:(

> 2. A different style button/icon from going detailed->compact.

How different? (in context of this patch/in global)?

yah, happy hacking :)
   J.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: magnifyGlass-small.png
Type: image/png
Size: 1008 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130108/ee79310d/magnifyGlass-small.png 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: redCross-small.png
Type: image/png
Size: 930 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130108/ee79310d/redCross-small.png 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cmpactDownloadIndicator.patch
Type: text/x-patch
Size: 12557 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130108/ee79310d/cmpactDownloadIndicator.patch