You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by michellephung <gi...@git.apache.org> on 2015/03/17 03:12:21 UTC
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
GitHub user michellephung opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/319
Fix Views toggle with dot symbol in ddoc name
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/michellephung/couchdb-fauxton Fix-views-toggle-with-dot-symbol-in-designdocname
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/319.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 #319
----
commit ad7215c6565a15507ab49153838f3e1cacd1e9ab
Author: michellephung@gmail.com <mi...@gmail.com>
Date: 2015-03-17T02:11:41Z
Fix Views toggle with dot symbol in ddoc name
----
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#discussion_r26569230
--- Diff: app/addons/documents/shared-views.js ---
@@ -160,12 +160,20 @@ function(app, FauxtonAPI, Components, Documents, Databases) {
className: "nav nav-list",
template: "addons/documents/templates/design_doc_menu",
events: {
- "click .js-collapse-toggle": "toggleArrow"
+ "click .js-collapse-toggle": "toggleArrow",
+ "click .js-toggle-submenu": 'toggleSubmenu'
},
toggleArrow: function(e){
this.$(e.currentTarget).toggleClass("down");
+ $('#' + this.model.id.replace(/^_design\//,"").replace(/\./g, '\\.')).collapse('toggle');
+ //if there is a '.' in the design doc name, jquery interprets that as a class
+ //see assets/js/libs/bootstrap.js line 1739
},
+ toggleSubmenu: function (e) {
+ this.$(e.currentTarget).toggleClass("down");
+ },
+
--- End diff --
please follow the codingguidelines:
newline between properties,
single quotes,
space after comma
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#issuecomment-88188557
@robertkowalski yikes! that's quite the test case :D
I can write the unit test, but before I do that I was wondering if what I'm doing is the best way to go about fixing the problem, or if this workaround adds more problems then it fixes, since it adds a bit of complexity to the what's being rendered in the html.
After some time thinking about this, I was thinking of optimizing my approach to the fix.
we are trying to show the url to follow the pattern : `database/mydbname/_design/myddocname/_view/myviewname`, but translating the document name to a url-safe string, can introduce '%', which breaks when used with jquery, and escaping %, would break the url
I was thinking of translating everything to a url-safe string, then take out % and replace it with `_percent_` for the html attributes that are used with jquery. Then anyone looking at the code wouldn't have to deal so much with the specific . / (space) issues, and they'd just have to deal with the one % special character.
any concerns? is there a better approach i'm overlooking?
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#discussion_r26669808
--- Diff: app/addons/documents/tests/nightwatch/checkSidebarBehavior.js ---
@@ -0,0 +1,34 @@
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+module.exports = {
+
+ 'Checks if design docs that have a dot symbol in the id show up in the UI': function (client) {
+ var waitTime = 10000,
+ newDatabaseName = client.globals.testDatabaseName,
+ designPrefix = '_design/',
+ newDocumentName1 = 'ddoc_normal',
+ newDocumentName2 = 'ddoc.with.specialcharacters',
+ baseUrl = client.globals.test_settings.launch_url;
+
+ client
+ .loginToGUI()
+ .createDocument(designPrefix + newDocumentName1, newDatabaseName)
+ .createDocument(designPrefix + newDocumentName2, newDatabaseName)
+ .url(baseUrl + '/#/database/' + newDatabaseName + '/_all_docs')
+ .clickWhenVisible('#nav-header-' + newDocumentName1)
+ .clickWhenVisible('#nav-header-' + newDocumentName2.replace(/\./g,'\\.'))
+ .waitForElementPresent('#' + newDocumentName1.replace(/\./g,'\\.'), waitTime, false)
+ .waitForElementPresent('#' + newDocumentName2.replace(/\./g,'\\.'), waitTime, false)
--- End diff --
please follow our coding guidelines (space after comma)
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#discussion_r26670361
--- Diff: app/addons/documents/shared-views.js ---
@@ -160,12 +160,21 @@ function(app, FauxtonAPI, Components, Documents, Databases) {
className: "nav nav-list",
template: "addons/documents/templates/design_doc_menu",
events: {
- "click .js-collapse-toggle": "toggleArrow"
+ "click .js-collapse-toggle": "toggleArrow",
+ 'click .js-toggle-submenu': 'toggleSubmenu'
},
toggleArrow: function(e){
- this.$(e.currentTarget).toggleClass("down");
+ this.$(e.currentTarget).toggleClass('down');
+ $('#' + this.model.id.replace(/^_design\//, '').replace(/\./g, '\\.')).collapse('toggle');
--- End diff --
we use `this.$`.
the fix does not work for me:
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/319
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#issuecomment-88291392
will take a look in detail tomorrow, but the approach using `_percent_` can create collisions, e.g. with a design doc called `design/number_of_users_percent_converted` that the user created or something like this
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#issuecomment-88290401
safeurl uses `encodeURIComponent`: https://github.com/apache/couchdb-fauxton/blob/9343084ace991386fab014529efdfdc67e10f389/app/core/utils.js#L98
So for converting back and forth we can use:
```js
decodeURIComponent(encodeURIComponent('_design/foo'));
```
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#discussion_r27466174
--- Diff: app/addons/documents/shared-views.js ---
@@ -184,12 +191,14 @@ function (app, FauxtonAPI, Components, Documents, Databases) {
serialize: function () {
var ddocName = this.model.id.replace(/^_design\//, ""),
docSafe = app.utils.safeURLName(ddocName),
- databaseName = this.collection.database.safeID();
+ databaseName = this.collection.database.safeID(),
+ data_target = app.utils.safeURLName(ddocName).replace(/%/g, '_percent_').replace(/\./g, '_dot_');
return {
- designDocMetaUrl: FauxtonAPI.urls('designDocs', 'app', databaseName, docSafe),
+ designDocMetaUrl: FauxtonAPI.urls('designDocs', 'app', databaseName, docSafe).replace(/\./g, '%2E'),
designDoc: ddocName,
ddoc_clean: docSafe,
+ data_target: data_target
--- End diff --
we still use camelCase
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by michellephung <gi...@git.apache.org>.
Github user michellephung commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#issuecomment-87879108
This is kind of a strange one.
So we are using data-target = "namewithspecialcharacters" with bootstrap and toggling the views.
Additionally, we are using the name of the view (with special characters) as the id / classes for that element in the view.
Jquery, (used with bootstrap), doesn't recognize a " " (space) or (slash) / or . (dot), so but you can escape them when using jquery to target the selector you want.
In this fix, I've escaped the special characters when it's used by jquery, and replaced them with "_nameofspecialcharacter_" in the elements where we need the id to be specifically not escaped (elements that are using data-target).
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#issuecomment-82242635
@michellephung looks good. We just need a passing test.
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#discussion_r26669889
--- Diff: app/addons/documents/shared-views.js ---
@@ -160,12 +160,21 @@ function(app, FauxtonAPI, Components, Documents, Databases) {
className: "nav nav-list",
template: "addons/documents/templates/design_doc_menu",
events: {
- "click .js-collapse-toggle": "toggleArrow"
+ "click .js-collapse-toggle": "toggleArrow",
+ 'click .js-toggle-submenu': 'toggleSubmenu'
--- End diff --
why do we need two event handlers now?
---
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.
---
[GitHub] couchdb-fauxton pull request: Fix Views toggle with dot symbol in ...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/319#issuecomment-88024157
hi michelle, this has a small bug: https://cloudup.com/ca92OopMbM1
how about writing some unit tests testing the single regexes and their combination?
---
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.
---