[rfc][icedtea-web] make download indicator more compact
Adam Domurad
adomurad at redhat.com
Mon Jan 7 11:00:03 PST 2013
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?
> 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.
There should be a slightly different icon from going from detailed
->compact.
>
>>
>> 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:
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.
2. A different style button/icon from going detailed->compact.
>
> Sounds reasonable, but definitely as different changeset.
>
>
> Thanx for review an ideas,
> J.
Happy hacking,
-Adam
More information about the distro-pkg-dev
mailing list