Bug 148730

The proposed class (standard copyright header is not included due to space saving):

package org.openide;

import java.awt.Color;
import javax.swing.ImageIcon;
import javax.swing.JLabel;
import javax.swing.UIManager;
import org.openide.util.ImageUtilities;
import org.openide.util.NbBundle;

/**
 * Helper class to simplify dealing with error/warning/info messages
 * @author Petr Slechta
 */
public class MsgHelper {

    private JLabel label;
    private Class<?> clazz;
    private static Color nbErrorForeground;
    private static Color nbWarningForeground;
    private static Color nbInfoForeground;
    private static ImageIcon errorIcon;
    private static ImageIcon warningIcon;
    private static ImageIcon infoIcon;

    /**
     * Static inicialization
     */
    static {
        nbErrorForeground = UIManager.getColor("nb.errorForeground"); //NOI18N
        if (nbErrorForeground == null)
            nbErrorForeground = new Color(255, 0, 0);

        nbWarningForeground = UIManager.getColor("nb.warningForeground"); //NOI18N
        if (nbWarningForeground == null)
            nbWarningForeground = new Color(0, 0, 0);

        nbInfoForeground = nbWarningForeground;

        errorIcon = new ImageIcon(ImageUtilities.loadImage("org/netbeans/modules/dialogs/error.png"));  //NOI18N
        warningIcon = new ImageIcon(ImageUtilities.loadImage("org/netbeans/modules/dialogs/warning.png"));  //NOI18N
        infoIcon = new ImageIcon(ImageUtilities.loadImage("org/netbeans/modules/dialogs/info.png"));  //NOI18N
    }
    
    /**
     * Creates new instance of MsgHelper
     * @param label JLabel component that is used for presentation of messages
     * @param clazz class that is used to localize message bundle used to get localized
     * version of messages
     */
    public MsgHelper(JLabel label, Class<?> clazz) {
        this.label = label;
        this.clazz = clazz;
    }

    /**
     * Set an error message
     * @param msgKey key from bundle that contains text of error message
     */
    public void setErrorMsg(String msgKey, Object... params) {
        label.setForeground(nbErrorForeground);
        label.setText(NbBundle.getMessage(clazz, msgKey, params));
        label.setIcon(errorIcon);
    }

    /**
     * Set an warning message
     * @param msgKey key from bundle that contains text of warning message
     */
    public void setWarningMsg(String msgKey, Object... params) {
        label.setForeground(nbWarningForeground);
        label.setText(NbBundle.getMessage(clazz, msgKey, params));
        label.setIcon(warningIcon);
    }

    /**
     * Set an informational message
     * @param msgKey key from bundle that contains text of info message
     */
    public void setInfoMsg(String msgKey, Object... params) {
        label.setForeground(nbInfoForeground);
        label.setText(NbBundle.getMessage(clazz, msgKey, params));
        label.setIcon(infoIcon);
    }

    /**
     * Clear message (no text and no icon are displayed)
     */
    public void clearMsg() {
        label.setText("");  //NOI18N
        label.setIcon(null);
    }
    
}

API reviewers, please review this proposal. Thanks!

JR01: What use-cases you are solving?
JR02: API documentation is missing?
JR03: Where are tests?

In general, submit patch rather then simple java source. Who will sustain this code? I disagree with changes like this
on the last time before HR. I'm going to change to ENHANCEMENT in question and solve later in next release.

Agreed, this needs more preparation.

Re JR01: What use-cases you are solving?
Please see Issues 148192 and 148405 for problems that could be solved with this new API.

Re JR02: API documentation is missing?
Yes, if approved, I will update also API documentation.

Re JR03: Where are tests?
There are no tests now. The class is very simple. If I write some tests, I will test Swing package most of the time...
:-) If you have any suggestions which tests should be written, please let me know.

I can sustain this class.

The change is very simple, so it could be done for 6.5. But if there are any objections, I'm OK to postpone this after
6.5 FCS...

MK1: The API change seems to be correct but is not conceptual IMHO. 
The settable Error text field shall be part of the Dialog/NotifyDescriptor and be eventually accessible through this
class. The workflow of adding a JLabel to the UI and later wrapping it into a helper class is not obvious.

Agreed with MK1, it would probably make sense to "sediment" this functionality from WizardDescriptor to DialogDescriptor.

See also issue 28862 (filed almost 6 years ago!) for more use cases. The proposal to sediment this functionality to
DialogDescriptor sounds reasonable, and this also suggests the answer to question "who will sustain this code" - it's a
standard part of the dialog framework, and should have been a long time ago.

This API should be considered for 7.0. Use of info icons should be consistent across all UI components, thus this API is
required. Otherwise each dialog will implement its own support and we will have many copy-pasted code sections across
all modules.

+1

My proposal is not approved, but there is a suggestion to sediment this functionality to DialogDescriptor. So
reassigning to component owner to handle it.

I will care about this. Once the final patch prepared, I'll take it back to apireviews.

Jiri,
it would be great if you can add this functionality in early stage of NB 7.0 development, so other developers may use
it. I personally know about 3 places where it can be used. Also several places may drop its own implementation of this
functionality and may use your new API...
Thanks!

I'm not still convinced of feasibility this proposal. We must be beware of forcing UI layout changes which couldn't be
backward compatible (Ccing Jano for get UI point of view) .The current design of Dialog API is based on the principle:
* API user designs UI component with own UI widgets which gives to the API as a parameter
* Dialogs API enlarge this given UI with buttons panel on the bottom of resulting dialog.
This proposal gives rise to re-layout of dialog with more enlarging of client's UI, which will be problem mainly in
dialogs which has no button on the bottom.
Possible solution:
1) brutal force, add (mainly) empty status line between buttons and client's UI
2) add new one constructor for such status line support available only for clients on demand
3) just add new methods like setError/Warning/InfoMessage() and hide it every time when empty.
Ad 1) unacceptable for backward compatibility reasons
Ad 2) pretty fine. However Dialog API descriptor has currently too many parameters constructo anyway.
Ad 3) AFAIK we don't like jumping size of dialogs which are according to visible/hidden status line.
So, the 2) seems as only feasible API change but under my reservations.

Btw. don't forget a Wizard API improvement since 7.8 (middle of NB6.5). It allows to set error/warning/info messages
just simple into wizards which covers many of use-cases of Dialogs API - i.e.
http://bits.netbeans.org/dev/javadoc/org-openide-dialogs/org/openide/WizardDescriptor.html#PROP_INFO_MESSAGE

Option 2) is fine for me.
It could be just one more parameter in constructor: boolean showStatusLine

Created attachment 74006 [details]
proposed patch

Reviewers, please review adding support for handling info/warning/error messages in NetBeans dialogs.
Petr S. could you apply the proposed change and make proof of concept on your use-cases and then attach code changes
containing usage this new API?
Thanks

looks ok in general to me.
MK1: how is html text supported? will the text wrap?

I will try to use the proposal to fix issues and let you know the result...

[JG01] The name empty() is odd. Perhaps clearMessages()?


[JG02] Javadoc mistake.

* To set such message use #createNotificationLineSupport
* @see createNotificationLineSupport

should be

* To set such message use {@link #createNotificationLineSupport}


[JG03] isNotificationLineSupportCreated does not seem useful to me. Who would call it?

MK1: HTML tagging is supported but the height of message area is fixed. Tooltip over message will render multiline messages.
JG01: Good. I'll rename the _empty()_ to _clearMessages()_ which makes good sense.
JG02: Done.
JG03: I agree isNotificationLineSupportCreated does not look useful but implementation(core.windows) of DialogDisplayer
needs to know this fact to layout UI of dialog correctly.

Jiri, I did test your changes and they are OK. I can use new API easily and it works fine.

[PS1] One minor issue: there should be some space between icon in notification line and dialog border (JLabel used for
notification line should have left and right border). See attached picture for illustration.

Please fix this minor issue, otherwise it is fine. Thanks!

Created attachment 74153 [details]
picture: no space between notification line and dialog border

PS1: I'll take care of it.
Thanks for review to everybody. I'm going to integrate this change tomorrow. If anybody doesn't agree, please speak up
now. Thanks

impl in core-main/rev/d9ef5112704b

Integrated into 'main-golden', will be available in build *200811281401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/d9ef5112704b
User: Jiri Rechtacek <jrechtacek@netbeans.org>
Log: #148730: Add helper class to simplify using error/warning/info messages in dialogs

v.

*** Bug 28862 has been marked as a duplicate of this bug. ***