Wicket
  1. Wicket
  2. WICKET-911

refactor feedback to allow users to easily add custom levels

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.3.0-final
    • Fix Version/s: None
    • Component/s: wicket
    • Labels:
      None

      Description

      we have preset levels of fatal/error/warn/info, but users keep asking for more, such as sucess, see WICKET-907. we should probably refactor feedback to have

      component.addfeedback(feedbackmessage) instead of having ten methods for prebuild levels. as far as i can see wicket doesnt need to know if the message is error or success so there is no point in having a more complicated system.

        Issue Links

          Activity

          Igor Vaynberg created issue -
          Igor Vaynberg made changes -
          Field Original Value New Value
          Link This issue blocks WICKET-907 [ WICKET-907 ]
          Igor Vaynberg made changes -
          Link This issue blocks WICKET-907 [ WICKET-907 ]
          Igor Vaynberg made changes -
          Link This issue is related to WICKET-907 [ WICKET-907 ]
          Igor Vaynberg made changes -
          Link This issue relates to WICKET-499 [ WICKET-499 ]
          Hide
          Eelco Hillenius added a comment -

          I'm fine with having a generic method, and I would be fine with removing fatal and warn, but I'd really like to keep error and info, partially because they are convenient, and partially because they'll be all over Wicket In Action.

          Show
          Eelco Hillenius added a comment - I'm fine with having a generic method, and I would be fine with removing fatal and warn, but I'd really like to keep error and info, partially because they are convenient, and partially because they'll be all over Wicket In Action.
          Hide
          Igor Vaynberg added a comment -

          this isnt scheduled till 1.4 so i dont think wia argument applies. convinience vs bloat we will have to evaluate when the time comes.

          Show
          Igor Vaynberg added a comment - this isnt scheduled till 1.4 so i dont think wia argument applies. convinience vs bloat we will have to evaluate when the time comes.
          Hide
          Will Hoover added a comment -

          I disagree... Wicket currently makes it convenient to generically define the message levels for underlying components that use them. For example, in a FeedbackPanel I can check the message level stored in the FeedbackMessage and determine how I need to format the message for display:

          protected final String getFeedbackResourcePrefix() {
          java.util.Iterator<FeedbackMessage> it = Session.get().getFeedbackMessages().iterator();
          FeedbackMessage fbm;
          int level = -1;
          while (it.hasNext()) {
          fbm = it.next();
          if (fbm.isFatal() || fbm.isError())

          { return "label.error"; }

          else if (fbm.isWarning())

          { level = FeedbackMessage.WARNING; }

          else if (level < FeedbackMessage.WARNING && fbm.isInfo())

          { level = FeedbackMessage.INFO; }

          }
          switch (level)

          { case FeedbackMessage.WARNING: return "label.warning"; case FeedbackMessage.INFO: return "label.info"; default: return "label.success"; }

          }

          However, I do see an advantage to removing the "bloat"... Why not remove all of the "error(message)", "info(message)", etc. and replace them with one method "message(message, level)" or "component.addFeedbackMessage(message, level)"? Then any message level is accommodated for and the message levels themselves can be removed from the FeedbackMessage class. The level will become just a placeholder.

          Show
          Will Hoover added a comment - I disagree... Wicket currently makes it convenient to generically define the message levels for underlying components that use them. For example, in a FeedbackPanel I can check the message level stored in the FeedbackMessage and determine how I need to format the message for display: protected final String getFeedbackResourcePrefix() { java.util.Iterator<FeedbackMessage> it = Session.get().getFeedbackMessages().iterator(); FeedbackMessage fbm; int level = -1; while (it.hasNext()) { fbm = it.next(); if (fbm.isFatal() || fbm.isError()) { return "label.error"; } else if (fbm.isWarning()) { level = FeedbackMessage.WARNING; } else if (level < FeedbackMessage.WARNING && fbm.isInfo()) { level = FeedbackMessage.INFO; } } switch (level) { case FeedbackMessage.WARNING: return "label.warning"; case FeedbackMessage.INFO: return "label.info"; default: return "label.success"; } } However, I do see an advantage to removing the "bloat"... Why not remove all of the "error(message)", "info(message)", etc. and replace them with one method "message(message, level)" or "component.addFeedbackMessage(message, level)"? Then any message level is accommodated for and the message levels themselves can be removed from the FeedbackMessage class. The level will become just a placeholder.
          Frank Bille Jensen made changes -
          Fix Version/s 1.4-M2 [ 12312911 ]
          Fix Version/s 1.4-M1 [ 12312523 ]
          Frank Bille Jensen made changes -
          Fix Version/s 1.4-M3 [ 12312912 ]
          Fix Version/s 1.4-M2 [ 12312911 ]
          Martijn Dashorst made changes -
          Fix Version/s 1.4-M3 [ 12312912 ]
          Fix Version/s 1.5-M1 [ 12313078 ]
          Igor Vaynberg made changes -
          Fix Version/s 1.5-M2 [ 12315237 ]
          Fix Version/s 1.5-M1 [ 12313078 ]
          Igor Vaynberg made changes -
          Fix Version/s 1.5-M3 [ 12315329 ]
          Fix Version/s 1.5-M2 [ 12315237 ]
          Jeremy Thomerson made changes -
          Fix Version/s 1.5-M4 [ 12315483 ]
          Fix Version/s 1.5-M3 [ 12315329 ]
          Martin Grigorov made changes -
          Fix Version/s 1.5-M4 [ 12315483 ]
          Hide
          Martijn Dashorst added a comment -

          Has been open for almost 5 years, no progress. Typically YAGNI

          Show
          Martijn Dashorst added a comment - Has been open for almost 5 years, no progress. Typically YAGNI
          Martijn Dashorst made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Won't Fix [ 2 ]
          Martijn Dashorst made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Unassigned
              Reporter:
              Igor Vaynberg
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development