You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by rakatyal <gi...@git.apache.org> on 2015/06/26 02:14:58 UTC
[GitHub] cordova-registry-web pull request: Raghav/routing
GitHub user rakatyal opened a pull request:
https://github.com/apache/cordova-registry-web/pull/23
Raghav/routing
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rakatyal/cordova-registry-web raghav/routing
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-registry-web/pull/23.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #23
----
commit f7926bdfb10704f8d7e2158695fe14ac6cb80c66
Author: Raghav Katyal <ra...@microsoft.com>
Date: 2015-06-25T22:19:22Z
Adding routing on search queries and auto url updations
commit 3714fc08708e32f68a5f58ca8ed9b82d98414ed5
Author: Raghav Katyal <ra...@microsoft.com>
Date: 2015-06-26T00:11:21Z
Adding trim to avoid extra space
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33378953
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
+window.onpopstate = function(e){
--- End diff --
should `window.addEventHandler` be used?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33378448
--- Diff: npm-search/assets/css/base.css ---
@@ -5,6 +5,11 @@ body {
margin: 0;
}
+h1 {
+ -webkit-margin-before: 0.3em;
--- End diff --
I added the <h1> tags on the page for SEO purposes. But the default margins above and below were huge and our page title didn't look good so I reduced it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33378042
--- Diff: npm-search/assets/css/base.css ---
@@ -5,6 +5,11 @@ body {
margin: 0;
}
+h1 {
+ -webkit-margin-before: 0.3em;
--- End diff --
Why is this change needed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33378891
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
--- End diff --
Why do we need a global variable here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33393473
--- Diff: npm-search/assets/js/app.js ---
@@ -280,12 +299,23 @@ var App = React.createClass({
}
else {
return {
- filterText: previousState.filterText + condition + ' ',
+ filterText: previousState.filterText.trim() + ' ' + condition + ' ',
plugins: previousState.plugins
};
}
});
},
+ loadFilterText : function(filterText) {
+ this.setState(function(previousState, currentProps) {
+ return {
+ filterText: filterText,
+ plugins: previousState.plugins
+ };
+ });
+ },
+ getURLParameter : function(name) {
+ return decodeURIComponent((new RegExp('[?|&]' + name + '=' + '([^&;]+?)(&|#|;|$)').exec(location.search)||[,""])[1].replace(/\+/g, '%20'))||null;
--- End diff --
consider adding a new line
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33394125
--- Diff: npm-search/assets/css/base.css ---
@@ -5,6 +5,11 @@ body {
margin: 0;
}
+h1 {
+ -webkit-margin-before: 0.3em;
--- End diff --
We should consider removing the wrapping altogether. Also, this change is webkit specific and likely does not work on IE.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33380810
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
+window.onpopstate = function(e){
--- End diff --
You mean bind this function to the onpopstate event using EventListener?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33397255
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
+window.onpopstate = function(e){
--- End diff --
okay will update
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33397998
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
--- End diff --
MAking it a member of PluginList class will make it a state variable meaning we will have to update it's state every time we render PluginList which is not really required.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33397305
--- Diff: npm-search/assets/css/base.css ---
@@ -5,6 +5,11 @@ body {
margin: 0;
}
+h1 {
+ -webkit-margin-before: 0.3em;
--- End diff --
yeah removing it and bringing the text to a single line centrally aligned with the image
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-registry-web/pull/23
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by rakatyal <gi...@git.apache.org>.
Github user rakatyal commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33380658
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
--- End diff --
We could have added it as a state variable to the plugin list component, but that would unnecessarily complicate stuff.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33393252
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
+window.onpopstate = function(e){
--- End diff --
yes, I meant `addEventListener` - that's just the best practice
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-registry-web pull request: Raghav/routing
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-registry-web/pull/23#discussion_r33393702
--- Diff: npm-search/assets/js/app.js ---
@@ -168,7 +168,15 @@ var Plugin = React.createClass({
</li>
)
}
-})
+});
+
+var timer=0;
--- End diff --
I see - Can timer be a member of `PluginList` class? Also, you might want to initialize to null instead of 0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org