Hi Sujith,
Thanks for the patches here is some code-review, it's mostly cosmetic
issues:
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=sujith/toaster-unique-projectname&id=17739189416e444d8dce7cb7c64aaf776f85d1ff
From 17739189416e444d8dce7cb7c64aaf776f85d1ff Mon Sep 17 00:00:00 2001
From: Sujith H <[email protected]>
Date: Mon, 25 Apr 2016 06:27:35 +0000
Subject: toaster: projectNameValidation API added
The projectNameValidation API would help users
to validate if a project name exists or not. This
API is added to libtoaster.
[YOCTO #7005]
Signed-off-by: Sujith H <[email protected]>
diff --git a/bitbake/lib/toaster/toastergui/static/js/libtoaster.js
b/bitbake/lib/toaster/toastergui/static/js/libtoaster.js
index 43930a2..e8c14cd 100644
--- a/bitbake/lib/toaster/toastergui/static/js/libtoaster.js
+++ b/bitbake/lib/toaster/toastergui/static/js/libtoaster.js
@@ -374,6 +374,46 @@ var libtoaster = (function (){
});
}
+ /* Validate project names. Use unique project names */
+ /* Arguments include projectnameid, hinterrorid, validateprojectnameid,
enableordisablebtnid */
- MW - Good that you're documenting the function but we need to know
what the arguments are, not just listing the variable names, that
doesn't really help as the names are already below in the function
definition anyway. We need to have a short description of what the
variables are expecting and the type if it's not obvious e.g. string or
jquery Element.
+ function _projectNameValidation(projectnameid, hinterrorid,
+ ctrlgrpvalidateprojectnameid, enableordisablebtnid ) {
+
- MW - The variables are all called somethingid, but you're not passing
an id, you're passing a jQuery element. Either is fine really but make
sure what you're passing in and the variable names match, e.g. if it's
going to be an id string "the-element-id" then it would be something
like projectNameElementId, or if you're going to pass in jQuery object
it could be projectName and then in the documentation string make sure
you say it's a jQuery object expected. Don't forget that in javascript
we use camelCaseLikeThis for variable names and functions.
https://wiki.yoctoproject.org/wiki/Contribute_to_Toaster#Javascript
We should also name the function something more like
makeProjectNameValidation because this function isn't going to return
anything, like 'invalid' or 'valid' as you'd expect from a validation
function, it is in fact setting up/making the event handlers for the inputs.
+ /* The moment user types project name remove the error */
+ projectnameid.on("input", function() {
+ ctrlgrpvalidateprojectnameid.removeClass('control-group error');
+ hinterrorid.hide();
+ });
+
+ /* Validate new project name */
+ projectnameid.on("blur",function() {
+ var projectname = $(this).val();
+ $.ajax({
+ type: "GET",
+ url: libtoaster.ctx.projectsTypeAheadUrl,
+ data: { 'search' : projectname },
+ headers: { 'X-CSRFToken' : $.cookie('csrftoken')},
+ success: function( data ) {
+ if (data.results.length > 0 &&
+ data.results[0].name === projectname) {
+ // This project name exists hence show the error and disable
+ // the save button
+ ctrlgrpvalidateprojectnameid.addClass('control-group error');
+ hinterrorid.show();
+ enableordisablebtnid.attr('disabled', 'disabled');
+ } else {
+ ctrlgrpvalidateprojectnameid.removeClass('control-group error');
+ hinterrorid.hide();
+ enableordisablebtnid.removeAttr('disabled');
+ }
+ },
+ error: function (data) {
+ console.log(data);
+ },
+ });
+ });
+ }
+
- MW - All this needs to be indented by 2 spaces as you're inside the
projectNameValidation function.
return {
reload_params : reload_params,
@@ -390,6 +430,7 @@ var libtoaster = (function (){
makeLayerAddRmAlertMsg : _makeLayerAddRmAlertMsg,
showChangeNotification : _showChangeNotification,
createCustomRecipe: _createCustomRecipe,
+ projectNameValidation: _projectNameValidation,
};
})();
--
cgit v0.10.2
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=sujith/toaster-unique-projectname&id=8d3b47ec15dbd93e077ec10cd928648258b84181
From 8d3b47ec15dbd93e077ec10cd928648258b84181 Mon Sep 17 00:00:00 2001
From: Sujith H <[email protected]>
Date: Mon, 25 Apr 2016 06:29:19 +0000
Subject: toaster: ui handles duplicate project name in new project page
When already existing project name is typed by user,
the ui pops up message regarding the existance of the
project name.
[YOCTO #7005]
Signed-off-by: Sujith H <[email protected]>
diff --git a/bitbake/lib/toaster/toastergui/templates/newproject.html
b/bitbake/lib/toaster/toastergui/templates/newproject.html
index e83b2be..19ab749 100644
--- a/bitbake/lib/toaster/toastergui/templates/newproject.html
+++ b/bitbake/lib/toaster/toastergui/templates/newproject.html
@@ -21,7 +21,10 @@
<fieldset>
<label>Project name <span class="muted">(required)</span></label>
- <input type="text" class="input-xlarge" required id="new-project-name"
name="projectname">
+ <div class="control-group" id="validate-project-name">
+ <input type="text" class="input-xlarge" required id="new-project-name"
name="projectname">
+ </br><span class="help-block error" style="display: none;"
id="hintError-project-name">A project with this name exists. Project names must be unique.</span>
+ </div>
</fieldset>
- MW - hint-error-project-name not hintError-project-name no camel case for
HTML elements
https://wiki.yoctoproject.org/wiki/Contribute_to_Toaster#HTML
<!--
<fieldset>
@@ -113,6 +116,13 @@
$('#description-' + new_release).fadeIn();
});
+ try {
+ libtoaster.projectNameValidation($("#new-project-name"),
$("#hintError-project-name"), $("#validate-project-name"), $(".btn-primary"));
+ } catch (e) {
+ document.write("Sorry, An error has occurred loading this page");
+ console.warn(e);
+ }
- MW - not sure why you've put this in a try catch? no need for this.
+
/* // Hide the project release when you select an analysis
project
function projectType() {
if ($("input[type='radio']:checked").val() ==
'build') {
--
cgit v0.10.2
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=sujith/toaster-unique-projectname&id=3d9d7a628b7280ecb62c7e05dc6a8c5bc0204792
From 3d9d7a628b7280ecb62c7e05dc6a8c5bc0204792 Mon Sep 17 00:00:00 2001
From: Sujith H <[email protected]>
Date: Mon, 25 Apr 2016 06:30:37 +0000
Subject: toaster: ui handles duplicate project name in project page
When already existing project name is typed by user,
the ui pops up message regarding the existance of the
project name. When an existing project is typed the save
button will be disabled. Else user can proceed ahead by
modifying the project name.
[YOCTO #7005]
Signed-off-by: Sujith H <[email protected]>
diff --git a/bitbake/lib/toaster/toastergui/templates/projecttopbar.html
b/bitbake/lib/toaster/toastergui/templates/projecttopbar.html
index 007de06..abf598e 100644
--- a/bitbake/lib/toaster/toastergui/templates/projecttopbar.html
+++ b/bitbake/lib/toaster/toastergui/templates/projecttopbar.html
@@ -9,6 +9,7 @@
try {
projectTopBarInit(ctx);
+ libtoaster.projectNameValidation($("#project-name-change-input"),
$("#hintError-project-name"), $("#validate-project-name"), $("#project-name-change-btn"));
- MW - This should be inside the projectTopBarInit function in projecttopbar.js
and not here. The projectTopBarInit function contains all the setup for the
project top bar, setting up the project input is part of that so should go
inside the function and not as a top level item. Generally we want as little
javascript in the html templates as possible as it makes it much harder to
debug and the lines between the templating engine and the client side logic
start to get blured
Generally we try also to stick to 80 cols, so if possible wrap this on top more
lines.
} catch (e) {
document.write("Sorry, An error has occurred loading this page");
console.warn(e);
@@ -33,11 +34,12 @@
{% endif %}
</h1>
<form id="project-name-change-form" style="margin-bottom: 0px; display:
none;">
- <div class="input-append">
+ <div class="input-append" id="validate-project-name">
<input class="huge input-xxlarge" type="text" id="project-name-change-input"
autocomplete="off" value="{{project.name}}">
<button id="project-name-change-btn" class="btn btn-large"
type="button">Save</button>
<a href="#" id="project-name-change-cancel" class="btn btn-large
btn-link">Cancel</a>
</div>
+ <span class="help-block error" style="display: none;"
id="hintError-project-name">A project with this name exists. Project names must be unique.</span>
</form>
</div>
--
cgit v0.10.2
Thanks,
Michael
On 19/04/16 15:14, Barros Pena, Belen wrote:
On 18/04/2016 12:53, "sujith h" <[email protected]> wrote:
On Mon, Apr 18, 2016 at 5:09 PM, sujith h
<[email protected]> wrote:
Hi Belen,
On Mon, Apr 18, 2016 at 4:57 PM, Barros Pena, Belen
<[email protected]> wrote:
On 14/04/2016 14:57, "[email protected] on behalf of sujith
h" <[email protected] on behalf of
[email protected]>
wrote:
On Thu, Mar 31, 2016 at 8:39 PM, Michael Wood
<[email protected]> wrote:
Done a quick review on IRC we'll try to convert the project page from
doing a POST directly to the view to use the ajax+ReST like approach we
have else where (e.g. custom image recipes).
Michael had once again reviewed my patch set today and I had trimmed down
my code:
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=sujith/toas
t
er-unique-projectname
Belen, can you check the branch again. I have deleted the branch and
created it again. And now the error doesn't come.
Yep: error is gone now, so I was able to try it :)
I've found a couple of small issues with the behaviour and the styling:
* we need to apply the .control-group and .error classes to the fieldset
containing the project name when the error message comes up, so that the
label and input field look red like the message. That will match what we
do in the rest of Toaster, and will help people realise there is an issue
with the name they typed.
* Michael tells me we are using the "change" event, and recommends we use
"blur" instead
* Finally, the error message should go away the moment the error condition
disappears, i.e. when users change the name. It needs to work exactly like
the validation for the layer name field in the import layer page. To see
it, create a project in Toaster with the master release, then click on
"import layer", then type "meta-oe" in the layer name and tab or click
away from the text field. A note will come up saying that a layer with
that name already exists. Then add / remove something from the layer name:
on key up the message will go away. This is exactly the behaviour we need
to use in the case of the project name, making things nicely consistent
across the UI
Finally, I am afraid my design is incomplete: you can change the name of
an existing project, so we need to validate the name is unique in that
case as well. I am really sorry: I should have noticed this. Luckily
Michael realised for me :)
I've updated the design document attached to the bug
https://bugzilla.yoctoproject.org/show_bug.cgi?id=7005
It now includes a second page explaining the missing bits. Once we have
things working in the new project page, hopefully it won't be too hard to
extend the functionality to the project configuration pages.
Thanks!
Belén
Thanks,
Sujith H
Thanks!
Belén
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=sujith/toa
s
ter-unique-projectname>
Michael
On 30/03/16 16:04, sujith h wrote:
Hi,
I have posted my changes for bug 7005:
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=sujith/toas
t
er-unique-projectname
<http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=sujith/toa
s
ter-unique-projectname> ( The top 2 commits ).
This patch set helps user not to have same project names for toaster.
Thanks,
Sujith H
--
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c
--
_______________________________________________
toaster mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/toaster
--
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c
--
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c
--
സുജിത് ഹരിദാസന്
Bangalore
<Project>Contributor to KDE project
<Project>Contributor to Yocto project
http://fci.wikia.com/wiki/Anti-DRM-Campaign
<Blog> http://sujithh.info
C-x C-c
--
_______________________________________________
toaster mailing list
[email protected]
https://lists.yoctoproject.org/listinfo/toaster
