You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Daniel Gornstein <dg...@mitre.org> on 2012/07/31 14:05:49 UTC
Review Request: Rave 654: Fixed empty comment is allowed after you add a new
gadget
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6240/
-----------------------------------------------------------
Review request for rave.
Description
-------
This patch does not allow empty comments to be posted on a widget in the store. If you try to post an empty comment an alert box pops up asking you to please enter text into the comment field.
Also added a line which clears out the comment field after posting a comment. Before, the same text you just submitted was still populated in the comment box.
Diffs
-----
trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js 1367061
Diff: https://reviews.apache.org/r/6240/diff/
Testing
-------
Thanks,
Daniel Gornstein
Re: Review Request: Rave 654: Fixed empty comment is allowed after you add a
new gadget
Posted by Raminder Singh <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6240/#review10427
-----------------------------------------------------------
i tried your changes and they are working well. only problem rightnow is to validate empty spaces. That is a problem with other pages also. I tried to trim space but i could not find how to show message in that case. Then it does not go to if. Any ideas to show a message in else? we can always add an alert as good old way :)
var comment = $.trim($("#newComment-"+widgetId).get(0).value);
- Raminder Singh
On Aug. 14, 2012, 5:02 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6240/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2012, 5:02 p.m.)
>
>
> Review request for rave.
>
>
> Description
> -------
>
> This patch does not allow empty comments to be posted on a widget in the store. If you try to post an empty comment an alert box pops up asking you to please enter text into the comment field.
>
> Also added a line which clears out the comment field after posting a comment. Before, the same text you just submitted was still populated in the comment box.
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/widget.jsp 1372847
> trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js 1372847
>
> Diff: https://reviews.apache.org/r/6240/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>
Re: Review Request: Rave 654: Fixed empty comment is allowed after you add a
new gadget
Posted by Daniel Gornstein <dg...@mitre.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6240/
-----------------------------------------------------------
(Updated Aug. 14, 2012, 5:02 p.m.)
Review request for rave.
Changes
-------
Changed patch to use html5 required tag to alert users that the comment field needs to be filled out, instead of an alert pop-up
Description
-------
This patch does not allow empty comments to be posted on a widget in the store. If you try to post an empty comment an alert box pops up asking you to please enter text into the comment field.
Also added a line which clears out the comment field after posting a comment. Before, the same text you just submitted was still populated in the comment box.
Diffs (updated)
-----
trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/widget.jsp 1372847
trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js 1372847
Diff: https://reviews.apache.org/r/6240/diff/
Testing
-------
Thanks,
Daniel Gornstein
Re: Review Request: Rave 654: Fixed empty comment is allowed after you add a new gadget
Posted by Raminderjeet Singh <ra...@gmail.com>.
+1 for handling both client and server side validations. I have a question related to handling validation on server side for REST services like WidgetApi. In case of Widget Comment we call WidgetApi.createWidgetComment. There is no validation done at server side right now. Response object is HttpServletResponse object where we can always say response.setError() with some message or do we have any other option. Also is it possible to show validation error like this with form objects?
Thanks
Raminder
On Aug 16, 2012, at 12:51 PM, Jasha Joachimsthal wrote:
>
>
> On 16 August 2012 17:42, Franklin, Matthew B. <mf...@mitre.org> wrote:
> On 8/14/12 1:07 PM, "Chris Geer" <ch...@cxtsoftware.com> wrote:
>
> >
> >
> >> On Aug. 7, 2012, 5:21 p.m., Raminder Singh wrote:
> >> > Patch works well . We dont have alert in rave and most of the
> >>validation are done by required attribute. HTML5 have
> >>required="required" for textarea also. Can we try using that? Thats
> >>better for mobile versions also.
> >
> >Question: I was researching the required field for some use in our
> >product and everything I've read says that support for it in Safari isn't
> >great and it only works on IE in version 10+. Since there are a lot of
> >people using Safari and IE < 10 how are they notified that a field is
> >required without another alert mechanism?
>
> I believe that Modernizr (script that we added) makes some of these
> features work in older browsers.
>
> The required attribute in the HTML should be a first, quick way to tell the user he has forgotten to fill in a field, even if you add a script for browsers that don't support it out of the box. The server should always check the submitted fields and if a required field is missing, it should display the form again with the proper feedback.
>
>
> >
> >
> >
> >- Chris
> >
> >
> >-----------------------------------------------------------
> >This is an automatically generated e-mail. To reply, visit:
> >https://reviews.apache.org/r/6240/#review9971
> >-----------------------------------------------------------
> >
> >
> >On Aug. 14, 2012, 5:02 p.m., Daniel Gornstein wrote:
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/6240/
> >> -----------------------------------------------------------
> >>
> >> (Updated Aug. 14, 2012, 5:02 p.m.)
> >>
> >>
> >> Review request for rave.
> >>
> >>
> >> Description
> >> -------
> >>
> >> This patch does not allow empty comments to be posted on a widget in
> >>the store. If you try to post an empty comment an alert box pops up
> >>asking you to please enter text into the comment field.
> >>
> >> Also added a line which clears out the comment field after posting a
> >>comment. Before, the same text you just submitted was still populated in
> >>the comment box.
> >>
> >>
> >> Diffs
> >> -----
> >>
> >>
> >>trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/widget.jsp
> >>1372847
> >>
> >>trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js
> >>1372847
> >>
> >> Diff: https://reviews.apache.org/r/6240/diff/
> >>
> >>
> >> Testing
> >> -------
> >>
> >>
> >> Thanks,
> >>
> >> Daniel Gornstein
> >>
> >>
> >
>
>
Re: Review Request: Rave 654: Fixed empty comment is allowed after
you add a new gadget
Posted by Jasha Joachimsthal <ja...@apache.org>.
On 16 August 2012 17:42, Franklin, Matthew B. <mf...@mitre.org> wrote:
> On 8/14/12 1:07 PM, "Chris Geer" <ch...@cxtsoftware.com> wrote:
>
> >
> >
> >> On Aug. 7, 2012, 5:21 p.m., Raminder Singh wrote:
> >> > Patch works well . We dont have alert in rave and most of the
> >>validation are done by required attribute. HTML5 have
> >>required="required" for textarea also. Can we try using that? Thats
> >>better for mobile versions also.
> >
> >Question: I was researching the required field for some use in our
> >product and everything I've read says that support for it in Safari isn't
> >great and it only works on IE in version 10+. Since there are a lot of
> >people using Safari and IE < 10 how are they notified that a field is
> >required without another alert mechanism?
>
> I believe that Modernizr (script that we added) makes some of these
> features work in older browsers.
>
The required attribute in the HTML should be a first, quick way to tell the
user he has forgotten to fill in a field, even if you add a script for
browsers that don't support it out of the box. The server should always
check the submitted fields and if a required field is missing, it should
display the form again with the proper feedback.
> >
> >
> >
> >- Chris
> >
> >
> >-----------------------------------------------------------
> >This is an automatically generated e-mail. To reply, visit:
> >https://reviews.apache.org/r/6240/#review9971
> >-----------------------------------------------------------
> >
> >
> >On Aug. 14, 2012, 5:02 p.m., Daniel Gornstein wrote:
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/6240/
> >> -----------------------------------------------------------
> >>
> >> (Updated Aug. 14, 2012, 5:02 p.m.)
> >>
> >>
> >> Review request for rave.
> >>
> >>
> >> Description
> >> -------
> >>
> >> This patch does not allow empty comments to be posted on a widget in
> >>the store. If you try to post an empty comment an alert box pops up
> >>asking you to please enter text into the comment field.
> >>
> >> Also added a line which clears out the comment field after posting a
> >>comment. Before, the same text you just submitted was still populated in
> >>the comment box.
> >>
> >>
> >> Diffs
> >> -----
> >>
> >>
> >>trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/widget.jsp
> >>1372847
> >>
> >>trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js
> >>1372847
> >>
> >> Diff: https://reviews.apache.org/r/6240/diff/
> >>
> >>
> >> Testing
> >> -------
> >>
> >>
> >> Thanks,
> >>
> >> Daniel Gornstein
> >>
> >>
> >
>
>
Re: Review Request: Rave 654: Fixed empty comment is allowed after
you add a new gadget
Posted by "Franklin, Matthew B." <mf...@mitre.org>.
On 8/14/12 1:07 PM, "Chris Geer" <ch...@cxtsoftware.com> wrote:
>
>
>> On Aug. 7, 2012, 5:21 p.m., Raminder Singh wrote:
>> > Patch works well . We dont have alert in rave and most of the
>>validation are done by required attribute. HTML5 have
>>required="required" for textarea also. Can we try using that? Thats
>>better for mobile versions also.
>
>Question: I was researching the required field for some use in our
>product and everything I've read says that support for it in Safari isn't
>great and it only works on IE in version 10+. Since there are a lot of
>people using Safari and IE < 10 how are they notified that a field is
>required without another alert mechanism?
I believe that Modernizr (script that we added) makes some of these
features work in older browsers.
>
>
>
>- Chris
>
>
>-----------------------------------------------------------
>This is an automatically generated e-mail. To reply, visit:
>https://reviews.apache.org/r/6240/#review9971
>-----------------------------------------------------------
>
>
>On Aug. 14, 2012, 5:02 p.m., Daniel Gornstein wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/6240/
>> -----------------------------------------------------------
>>
>> (Updated Aug. 14, 2012, 5:02 p.m.)
>>
>>
>> Review request for rave.
>>
>>
>> Description
>> -------
>>
>> This patch does not allow empty comments to be posted on a widget in
>>the store. If you try to post an empty comment an alert box pops up
>>asking you to please enter text into the comment field.
>>
>> Also added a line which clears out the comment field after posting a
>>comment. Before, the same text you just submitted was still populated in
>>the comment box.
>>
>>
>> Diffs
>> -----
>>
>>
>>trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/widget.jsp
>>1372847
>>
>>trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js
>>1372847
>>
>> Diff: https://reviews.apache.org/r/6240/diff/
>>
>>
>> Testing
>> -------
>>
>>
>> Thanks,
>>
>> Daniel Gornstein
>>
>>
>
Re: Review Request: Rave 654: Fixed empty comment is allowed after you add a
new gadget
Posted by Chris Geer <ch...@cxtsoftware.com>.
> On Aug. 7, 2012, 5:21 p.m., Raminder Singh wrote:
> > Patch works well . We dont have alert in rave and most of the validation are done by required attribute. HTML5 have required="required" for textarea also. Can we try using that? Thats better for mobile versions also.
Question: I was researching the required field for some use in our product and everything I've read says that support for it in Safari isn't great and it only works on IE in version 10+. Since there are a lot of people using Safari and IE < 10 how are they notified that a field is required without another alert mechanism?
- Chris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6240/#review9971
-----------------------------------------------------------
On Aug. 14, 2012, 5:02 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6240/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2012, 5:02 p.m.)
>
>
> Review request for rave.
>
>
> Description
> -------
>
> This patch does not allow empty comments to be posted on a widget in the store. If you try to post an empty comment an alert box pops up asking you to please enter text into the comment field.
>
> Also added a line which clears out the comment field after posting a comment. Before, the same text you just submitted was still populated in the comment box.
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/webapp/WEB-INF/jsp/views/widget.jsp 1372847
> trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js 1372847
>
> Diff: https://reviews.apache.org/r/6240/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>
Re: Review Request: Rave 654: Fixed empty comment is allowed after you add a
new gadget
Posted by Raminder Singh <ra...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6240/#review9971
-----------------------------------------------------------
Patch works well . We dont have alert in rave and most of the validation are done by required attribute. HTML5 have required="required" for textarea also. Can we try using that? Thats better for mobile versions also.
- Raminder Singh
On July 31, 2012, 12:05 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6240/
> -----------------------------------------------------------
>
> (Updated July 31, 2012, 12:05 p.m.)
>
>
> Review request for rave.
>
>
> Description
> -------
>
> This patch does not allow empty comments to be posted on a widget in the store. If you try to post an empty comment an alert box pops up asking you to please enter text into the comment field.
>
> Also added a line which clears out the comment field after posting a comment. Before, the same text you just submitted was still populated in the comment box.
>
>
> Diffs
> -----
>
> trunk/rave-portal-resources/src/main/webapp/static/script/rave_store.js 1367061
>
> Diff: https://reviews.apache.org/r/6240/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>