[Dancer-users] Flash Message
Flavio Poletti
polettix at gmail.com
Fri Jan 28 19:05:06 CET 2011
On Fri, Jan 28, 2011 at 6:52 PM, damien krotkine <dkrotkine at gmail.com>wrote:
> On 28 January 2011 18:40, Flavio Poletti <polettix at gmail.com> wrote:
> > Hi,
> >
> > the implementation is sound, even though I'd prefer to be able to set
> > multiple messages per single channel. If I validate a form, for example,
> I'd
> > like to flash a warning for each field I don't like, not just the last
> one.
>
> Hm, my brain being what it is on friday night, could you elaborate a
> bit more ? can you give an example of such channels ?
>
> I'm thinking you could use an array of string as flash.warnings,
> instead of a simple string, but I'm not sure I understand what you're
> saying
>
Sorry, you got the "channel" idea. I'd like to use flash like this:
flash warning => 'you made error X';
and then, possibly later, say:
flash warning => 'you made error Y';
In the current implementation, only the last gets to the template. I'm
suggesting to always push the incoming $value into an array. Otherwise, with
the current implementation that lets you delete stuff, I'd be forced to do
it like this:
my $array = flash('warning') || [];
push @$array, 'you made error X';
flash warning => $array;
which is a bit ugly...
>
> > On the other hand, deleting flash messages from the controller does seem
> to
> > be a corner case, which I'd handle with an ad-hoc function or ignore
> > completely.
> >
> > I'll try to propose something on GitHub that lets the end user decide the
> > preferred semantics through a configuration; I would personally like to
> > avoid having leftovers in the flash when my templates are not bug-free
> and I
> > definitely don't like the idea of remembering to call flash->flush()
>
> I agree. After all, maybe it would make sense to have 3 behaviors :
> behaviour 1 : delete all the keys as soon as one is used (to be implemented
> )
> behaviour 2 : delete only the key which is used ( current implementation )
> behaviour 3 : never delete the keys ( currently persistence = 1 )
>
> And maybe behaviour 1 should be the default, I don't know. As long as
> behaviour 2 is available, I'm happy (and brian too)
>
> maybe the behaviour could be chosen with the persistence config
> keyword ? It could be renamed to flush_behaviour
>
> flush_behaviour = key_level
> flush_behaviour = hash_level
> flush_behaviour = never
>
> What do you think ? OK I'm terrible at finding good names, so any help
> welcome :)
I'd leave "_behaviour", add an 'always' (your previous implementation) and
put something like:
flush = 'key'
flush = 'all'
flush = 'always'
flush = 'never'
Anyway, as long as it's documented and it's reasonably connected to the
behaviour everything is fine :-)
>
>
> > (even
> > though it sounds so nice!)
>
> aha, yes, I like flash->flush :) Let's add it for fun, even if it's
> not necessary :)
>
>
> >
> > Cheers,
> >
> > Flavio.
> >
> > On Fri, Jan 28, 2011 at 6:33 PM, damien krotkine <dkrotkine at gmail.com>
> > wrote:
> >>
> >> I agree with Brian, for a slightly different reason :
> >>
> >> Let's imagine this:
> >>
> >> you have a Template1, that does this : <% flash.keys() %>, to be able
> >> to know that they are errors. If that's the case, it doesn't display
> >> the errors, but display a link, saying that they are erros. This link
> >> does a new request against the server, serving Template2
> >>
> >> Template2 does use flash.error, and effectively consumes the message.
> >>
> >> You don't want the flash hash structure to be emptied just after
> >> having displayed Template1, you want the messages to survive until
> >> Template2 (after all, you didn't consume them in Template1)
> >>
> >> If I understand correctly, In the original implementation, the simple
> >> fact of getting the keys of the hash would delete all the messages, so
> >> the next request, Template2 won't be able to retrieve <% flash.error
> >> %>
> >>
> >> In my new implementation, it would work.
> >>
> >> Granted, this is a bit of a corner case, however, I don't see any
> >> drawback.
> >>
> >> I could also add a flash->flush() method.
> >>
> >>
> >> On 28 January 2011 18:09, Brian E. Lozier <brian at massassi.com> wrote:
> >> > I can't think of any real world use case where you'd only want to show
> >> > certain messages. That said, it still seems better to only clear the
> >> > messages you use, not all of them.
> >> >
> >> > On Fri, Jan 28, 2011 at 9:02 AM, Flavio Poletti <polettix at gmail.com>
> >> > wrote:
> >> >> Hi,
> >> >>
> >> >> of course we can try to adapt to it (it seems a bit difficult
> >> >> although
> >> >> probably not impossible), but I fail to see the point.
> >> >>
> >> >> IMHO, if you get the chance to show the messages, you should show
> them
> >> >> all:
> >> >> showing warnings in one page and errors in the following one would
> >> >> puzzle
> >> >> the end user, wouldn't it?
> >> >>
> >> >> Unless there is some use case that point in the opposite direction
> I'd
> >> >> stick
> >> >> to the "wipe them all when you get the chance to show them"
> semantics.
> >> >>
> >> >> Cheers,
> >> >>
> >> >> Flavio.
> >> >>
> >> >> On Fri, Jan 28, 2011 at 5:32 PM, Brian E. Lozier <brian at massassi.com
> >
> >> >> wrote:
> >> >>>
> >> >>> It looks like a pretty good solution. The only thing I can see
> would
> >> >>> be that if you are in a template and check for flash.error but not
> >> >>> flash.warning, it looks like it will clear both flash.error and
> >> >>> flash.warning. It seems like it should only clear the one you're
> >> >>> checking for.
> >> >>>
> >> >>>
> >> >>>
> >> >>> On Fri, Jan 28, 2011 at 12:02 AM, Flavio Poletti <
> polettix at gmail.com>
> >> >>> wrote:
> >> >>> > Hi,
> >> >>> > I thought a bit about it and I think that the current
> >> >>> > implementation
> >> >>> > is a
> >> >>> > bit too limited (this seems also what brian thinks in the bug
> >> >>> > report http://rt.cpan.org/Public/Bug/Display.html?id=65009 - at
> >> >>> > least
> >> >>> > from
> >> >>> > what I understand).
> >> >>> > The problem is that the flash message might have to be displayed
> in
> >> >>> > the
> >> >>> > next
> >> >>> > request, which happens when redirections kick in. In this case,
> the
> >> >>> > flash
> >> >>> > message should be set, kept in the session and removed only when
> it
> >> >>> > is
> >> >>> > actually displayed; this should happen automatically, i.e. without
> >> >>> > playing
> >> >>> > with the $persistent variable - which is global - or with further
> >> >>> > overhead
> >> >>> > on the controller side.
> >> >>> > Following brian's feedback, I tried to re-code the plugin with the
> >> >>> > following
> >> >>> > criteria:
> >> >>> > * the "flash" method only adds messages (and handles many of them
> >> >>> > for
> >> >>> > every
> >> >>> > category - you might have more than one error in a page). I see no
> >> >>> > point
> >> >>> > in
> >> >>> > removing messages while inside the controller;
> >> >>> > * the message removal logic is shifted in the template via an
> >> >>> > anonymous
> >> >>> > sub.
> >> >>> > If the sub is called then the flash cache is cleared, otherwise it
> >> >>> > remains
> >> >>> > there. This should guarantee that the messages are not removed
> until
> >> >>> > they
> >> >>> > get the chance to be displayed.
> >> >>> > This is the core of the recoding - not tested but should suffice
> to
> >> >>> > give
> >> >>> > you
> >> >>> > the idea:
> >> >>> > register flash => sub {
> >> >>> > my ($key, $value) = @_;
> >> >>> > my $flash = session($session_hash_key);
> >> >>> > if (! $flash) { # initialise the container for flash messages
> >> >>> > $flash = {};
> >> >>> > session($session_hash_key, $flash);
> >> >>> > }
> >> >>> > push @{$flash->{$key}}, $value;
> >> >>> > return;
> >> >>> > };
> >> >>> > before_template sub {
> >> >>> > my $obj = shift;
> >> >>> > my $flashes;
> >> >>> > $obj->{$token_name} = sub {
> >> >>> > if (! $flashes) { # first call, get messages and clear
> >> >>> > $flashes = session($session_hash_key) || {};
> >> >>> > session($session_hash_key, {});
> >> >>> > }
> >> >>> > return $flashes;
> >> >>> > };
> >> >>> > return;
> >> >>> > };
> >> >>> >
> >> >>> > IMHO there is no need for an "exists", in the template you can do
> >> >>> > like
> >> >>> > this:
> >> >>> > <% IF flash.errors %>...<% END %>
> >> >>> > The important thing to remember is that even checking for the
> >> >>> > presence
> >> >>> > of
> >> >>> > something inside "flash" clears the messages, which is consistent
> >> >>> > with
> >> >>> > the
> >> >>> > semantics that "they get cleared as soon as they have a chance to
> be
> >> >>> > displayed". The $flashes variable guarantees that they survive at
> >> >>> > least
> >> >>> > until the end of the request, i.e. you are able to call "flash"
> >> >>> > multiple
> >> >>> > times while handling a request and always get all messages.
> >> >>> > If you think that this makes sense, I can propose a pull request
> on
> >> >>> > GitHub
> >> >>> > or you can make the changes directly.
> >> >>> > Cheers,
> >> >>> > Flavio.
> >> >>> >
> >> >>> >
> >> >>> > On Tue, Jan 11, 2011 at 5:48 PM, damien krotkine
> >> >>> > <dkrotkine at gmail.com>
> >> >>> > wrote:
> >> >>> >>
> >> >>> >> I've re-implemented it to be more Rails-like, as sukria said.
> >> >>> >>
> >> >>> >> https://github.com/dams/Dancer-Plugin-FlashMessage
> >> >>> >>
> >> >>> >> and on CPAN, pending mirrors refresh.
> >> >>> >>
> >> >>> >> The funny part of the story ? the effective code is only 30 lines
> >> >>> >> long. Talking about Perl and Dancer expressiveness...
> >> >>> >>
> >> >>> >> dams.
> >> >>> >>
> >> >>> >> On 11 January 2011 14:40, Alexis Sukrieh <sukria at sukria.net>
> wrote:
> >> >>> >> > Hi list!
> >> >>> >> >
> >> >>> >> > Le 11/01/2011 14:29, damien krotkine a écrit :
> >> >>> >> >>
> >> >>> >> >> Hi,
> >> >>> >> >>
> >> >>> >> >> following previous thread, I've done a first implementation of
> >> >>> >> >> Dancer::Plugin::FlashMessage :
> >> >>> >> >>
> >> >>> >> >> https://github.com/dams/Dancer-Plugin-FlashMessage
> >> >>> >> >
> >> >>> >> > Great! Thanks a lot for your time dams, the myth is still
> alived!
> >> >>> >> > (Dancer's
> >> >>> >> > community)++
> >> >>> >> >
> >> >>> >> >
> >> >>> >> >
> >> >>> >> >> Some parts need to be improved, for instance :
> >> >>> >> >>
> >> >>> >> >> - it supports only one flash message
> >> >>> >> >> - the keywords are not short enough.
> >> >>> >> >>
> >> >>> >> >> So I think I'll change the implementation so that the template
> >> >>> >> >> token
> >> >>> >> >> is
> >> >>> >> >> simply called 'flash', and it'll be a hash, like in Rails.
> I'll
> >> >>> >> >> also
> >> >>> >> >> change the registered method so that it's just flash() instead
> >> >>> >> >> of
> >> >>> >> >> get_flash()
> >> >>> >> >
> >> >>> >> > I agree. I'd like to behave just like Rails' flash feature. The
> >> >>> >> > idea
> >> >>> >> > is
> >> >>> >> > pretty straight forward:
> >> >>> >> >
> >> >>> >> > "flash" is an accessor to a particular session hash table whose
> >> >>> >> > values
> >> >>> >> > can
> >> >>> >> > only be accessed once. Nothing more complicated than that.
> >> >>> >> >
> >> >>> >> > So to conclude, IMO, flash should be a wrapper like the
> >> >>> >> > following:
> >> >>> >> >
> >> >>> >> > sub flash {
> >> >>> >> > my ($key, $value) = @_;
> >> >>> >> > my $flash = session('_flash');
> >> >>> >> >
> >> >>> >> > # write
> >> >>> >> > if (@_ == 2) {
> >> >>> >> > $flash->{$key} = $value;
> >> >>> >> > session('_flash' => $flash);
> >> >>> >> > }
> >> >>> >> >
> >> >>> >> > # read (+ delete)
> >> >>> >> > else {
> >> >>> >> > my $value = $flash->{$key};
> >> >>> >> > delete $flash->{$key} if defined $value;
> >> >>> >> > session('_flash' => $flash);
> >> >>> >> > }
> >> >>> >> >
> >> >>> >> > return $value;
> >> >>> >> > }
> >> >>> >> >
> >> >>> >> > This is it, I think. This allows for the following code in a
> >> >>> >> > Dancer
> >> >>> >> > app:
> >> >>> >> >
> >> >>> >> >
> >> >>> >> > get '/' => sub {
> >> >>> >> > flash welcome => "This is a welcome message, only shown
> >> >>> >> > once";
> >> >>> >> > }
> >> >>> >> >
> >> >>> >> > Then, as soon as the key 'welcome' is accesed via
> >> >>> >> > flash('welcome'),
> >> >>> >> > the
> >> >>> >> > entry will be purged.
> >> >>> >> >
> >> >>> >> > This will be very helpful for authentication stuff in before
> >> >>> >> > filters,
> >> >>> >> > error
> >> >>> >> > messages, notifications, ....
> >> >>> >> >
> >> >>> >> >
> >> >>> >> > Kudos to dams!
> >> >>> >> >
> >> >>> >> > (BTW I haven't read the code yet)
> >> >>> >> >
> >> >>> >> > --
> >> >>> >> > Alexis Sukrieh
> >> >>> >> > _______________________________________________
> >> >>> >> > Dancer-users mailing list
> >> >>> >> > Dancer-users at perldancer.org
> >> >>> >> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
> >> >>> >> >
> >> >>> >> _______________________________________________
> >> >>> >> Dancer-users mailing list
> >> >>> >> Dancer-users at perldancer.org
> >> >>> >> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
> >> >>> >
> >> >>> >
> >> >>> > _______________________________________________
> >> >>> > Dancer-users mailing list
> >> >>> > Dancer-users at perldancer.org
> >> >>> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
> >> >>> >
> >> >>> >
> >> >>> _______________________________________________
> >> >>> Dancer-users mailing list
> >> >>> Dancer-users at perldancer.org
> >> >>> http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
> >> >>
> >> >>
> >> > _______________________________________________
> >> > Dancer-users mailing list
> >> > Dancer-users at perldancer.org
> >> > http://www.backup-manager.org/cgi-bin/listinfo/dancer-users
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.backup-manager.org/pipermail/dancer-users/attachments/20110128/b413905b/attachment-0001.htm>
More information about the Dancer-users
mailing list