You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2015/11/25 14:33:43 UTC
[GitHub] couchdb-fauxton pull request: Table view: add prioritized table vi...
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/580
Table view: add prioritized table view
- offer a truncated and a filtered table view
- filtered table viewis initially prioritized from the most
occuring fields
- offer dropdown / typeahead to select which field should be used
for filtering
- add small working improvements
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-fauxton filter-table
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/580.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 #580
----
commit 460ead9cefab58d745aa3cb3acd91aa15d93fff3
Author: Robert Kowalski <ro...@apache.org>
Date: 2015-11-25T13:29:25Z
add react-autocomplete
src: https://github.com/rackt/react-autocomplete/blob/master/dist/react-autocomplete.js
version: 412b122 on 12 Aug
commit 63fccf88ae1b176b1e546d8ef189fda9c87fbec6
Author: Robert Kowalski <ro...@apache.org>
Date: 2015-11-25T13:31:08Z
add prioritized table view
- offer a truncated and a filtered table view
- filtered table viewis initially prioritized from the most
occuring fields
- offer dropdown / typeahead to select which field should be used
for filtering
- add small working improvements
----
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-160181391
Regarding the checkboxes not all checking, I did a little debugging and it seems to be caused by the ID being encoded. To reproduce:
- create a doc with an ID of `neat:one-two-three`
- go back to the all docs page. Switch to table view and select all.
Result: doc isn't selected.
Looks like the hyphen causes the ID to be encoded somewhere down the line, which encodes the `:` char, causing it to fail to be checked.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r45893297
--- Diff: app/addons/documents/index-results/index-results.components.react.jsx ---
@@ -216,6 +368,7 @@ function (app, FauxtonAPI, React, Stores, Actions, Components, Documents) {
getDocumentStyleView: function (loadLines) {
var classNames = 'view';
+ var isDeletable = this.props.isListDeletable;
--- End diff --
This used? Looks like it can be dropped.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r48768820
--- Diff: app/addons/documents/index-results/index-results.components.react.jsx ---
@@ -73,21 +84,37 @@ function (app, FauxtonAPI, React, Stores, Actions, Components, Documents) {
return row;
},
+ maybeGetSpecialField: function (element, i) {
+ if (!this.props.data.hasMetadata) {
+ return null;
+ }
+
+ var el = element.content;
+ var key = 'tableview-data-cell-special' + i;
--- End diff --
`key` isn't 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.
---
[GitHub] couchdb-fauxton pull request: Table view: add prioritized table vi...
Posted by justin-mcdavid-ibm <gi...@git.apache.org>.
Github user justin-mcdavid-ibm commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159693218
Inverted selection sounds like buggy behavior. The version of this I was playing with had normal bulk-selector behavior.
Include docs could be brought out, but I don't understand why we'd do away with the Table All view.
I'd like to see the Displaying/Showing messages consolidated in the footer. I've already talked to Robert about changes to the strings: "Showing document 21 - 40. Showing 5 of 13 table columns." And then on the right side, it would need to say “Documents per page:” rather than “Per Page:"
We don't have functionality to support the down arrow by the bulk selector. Currently there's only the ability to delete selected docs. I'd like to see the bulk selector moves to the top left of the table, to better use screen real estate:
But then we run into issue of the clipboard icon and trashcan Icon being more closely aligned, and looking rather similar. The trashcan does trigger a confirmation dialogue box, though. I need to think about this one a bit.
The filter icon should probably only show up when a column field has been cleared, so that we can still maintain as much horizontal space for values as we can.
Anyway, a lot of these things could be handled in another PR.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-169784699
This looks good, Robert. +1 Nice work getting that cache removed.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-170616100
I found a few small things.
- "document" shouldn't be capitalized in the copy-to-clipboard msg.
<img width="478" alt="screen shot 2016-01-11 at 8 41 57 am" src="https://cloud.githubusercontent.com/assets/512116/12239792/4a22c148-b83f-11e5-8634-2ea9d966bc9d.png">
- This is an old issue (feel free to omit in this ticket) but the first text string in the footer isn't vertically aligned with the others in the row.
<img width="1014" alt="screen shot 2016-01-11 at 8 42 56 am" src="https://cloud.githubusercontent.com/assets/512116/12239814/680b1d5e-b83f-11e5-81d1-266f6a79b089.png">
- This is an odd one. Small behaviour bug:
1. In a database that has > 5 items, select 5 docs per page.
2. Select a doc on the first page and go to the next page.
3. Select a doc on that page.
4. Click back to page 1, note that originally selected doc was still selected [good!]
5. Click to page 2 again. The document one page 2 isn't selected.
Expectation: the doc on page 2 would still be selected like the doc on page 1.
- Another odd behaviour issue:
1. With `include docs` unchecked, select a document on a page.
2. Click `include docs`. Note that the doc you'd selected isn't selected any more (this is fine, I guess: but I'd prefer it to be still selected myself)
3. Click a different doc.
Result: the new row appears checked, as does the old checked row as well.
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/580
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r48768434
--- Diff: app/addons/documents/header/header.react.jsx ---
@@ -41,101 +52,60 @@ function (app, FauxtonAPI, React, Actions, ReactComponents, IndexResultsStore, I
componentDidMount: function () {
indexResultsStore.on('change', this.onChange, this);
+ queryOptionsStore.on('change', this.onChange, this);
+
},
componentWillUnmount: function () {
indexResultsStore.off('change', this.onChange);
+ queryOptionsStore.off('change', this.onChange);
},
onChange: function () {
this.setState(this.getStoreState());
},
- getIcon: function () {
- if (this.state.selectedView === 'table') {
- return 'fonticon-table';
- }
-
- if (this.state.selectedView === 'collapsed') {
- return 'fonticon-list-alt';
- }
-
- return 'fonticon-json';
- },
-
- getCollapseDocsButton: function () {
- var icon = this.getIcon();
-
- return (
- <div className="add-dropdown">
- <div className="dropdown">
- <button data-toggle="dropdown" className="button header-control-box control-view">
- <i className={"dropdown-toggle icon " + icon}></i> View
- </button>
- <ul className="dropdown-menu arrow" role="menu" aria-labelledby="dLabel">
- <li>
- <a onClick={this.collapseAllDocuments}>
- <i className="fonticon-list-alt" />
- <div>
- Collapsed View
- </div>
- </a>
- </li>
- <li>
- <a onClick={this.toggleToNormalJson}>
- <i className="fonticon-json" />
- <div>
- Expanded View
- </div>
- </a>
- </li>
- <li>
- <a onClick={this.tablelizeView}>
- <i className="fonticon-table" />
- <div>
- Table View
- </div>
- </a>
- </li>
- </ul>
-
- </div>
- </div>
- );
- },
-
render: function () {
+ var isTableViewSelected = this.state.isTableView;
+
return (
- <div className='alternative-header'>
- {this.getCollapseDocsButton()}
+ <div className="alternative-header">
+ <ButtonGroup className="header-toggle-button">
+ <Button
+ className={isTableViewSelected ? '' : 'active'}
+ onClick={this.toggleTableView.bind(this, false)}
+ >
+ <i className="fonticon-json" /> JSON
+ </Button>
+ <Button
+ className={isTableViewSelected ? 'active' : ''}
+ onClick={this.toggleTableView.bind(this, true)}
+ >
+ <i className="fonticon-table" /> Table
+ </Button>
+ </ButtonGroup>
+ {this.props.showIncludeAllDocs ? <ToggleHeaderButton
+ toggleCallback={this.toggleIncludeDocs}
+ containerClasses="header-control-box control-toggle-include-docs"
+ title="Enable/Disabe include_docs"
--- End diff --
Typo. Should be `Disable`.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r45892455
--- Diff: app/addons/documents/header/header.actions.js ---
@@ -42,9 +42,10 @@ function (app, FauxtonAPI, ActionTypes) {
});
},
- enableTableView: function () {
+ enableTableView: function (displayType) {
FauxtonAPI.dispatch({
- type: ActionTypes.ENABLE_TABLE_VIEW
+ type: ActionTypes.ENABLE_TABLE_VIEW,
+ displayType: displayType
--- End diff --
Not a big deal, but how do you feel about always using an `options` key here? i.e. putting all payload content into an `options` sub-object. I like it because it differentiates "special" keys (really only `type` right now) from arbitrary message content. Seems like a good gamble for down the road.
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159678560
note: i just found out that fauxton supported "Update Sequences" in the footer but they were not displayed because of css issues. it looks like the 2.0 api does not return them any more, too
---
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: Table view: add prioritized table vi...
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/580#discussion_r47237432
--- Diff: app/addons/documents/index-results/actions.js ---
@@ -96,7 +104,7 @@ function (app, FauxtonAPI, ActionTypes, Stores, Documents, SidebarActions) {
collection: collection,
query: options.queryCode,
textEmptyIndex: 'No Results Found!',
- bulkCollection: Documents.BulkDeleteDocCollection
+ bulkCollection: bulkCollection
--- End diff --
i really got into trouble here.
the collection is stores in the store, and received by `indexResultsStore.getCollection()` -- the same collection is put into the paginationStore to handle page updates.
both stores and actions (paginationStore, index-resultStore, paginationAction, paginationStore) alter the collection and it really got hairy here.
in this file i tried the approach of the full roundtrip with the bulkDeleteCollection in `deleteSelected ` (scroll down) - but not happy with it either (easier to reason about, but too much code too change right now as it is used this way everywhere) - apart from that i am happy with it, maybe just do the work 1 time, what do you think?
any other alternatives?
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-171093048
Drat, sorry @robertkowalski, one more thing: the toggle-all action is inverting the selection on the table and JSON views. It should just simply enable/disable all. Justin confirmed this behaviour in his first response in this thread above.
Other than that is all looks good!
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-171490575
+1 :fireworks: :tada: :balloon: :laughing:
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-160148137
@benkeen @justin-mcdavid-ibm
i could imagine just to show the table (truncated) button just if include_docs is enabled, that way it would not confuse people?
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-170036949
@justin-mcdavid-ibm can you point out the differences for me, i was not able to spot them by testing
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-171913720
merged! thnaks everyone!
---
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: Table view: add prioritized table vi...
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/580#discussion_r47236292
--- Diff: app/addons/documents/index-results/stores.js ---
@@ -176,33 +195,31 @@ function (app, FauxtonAPI, ActionTypes, HeaderActionTypes, Documents, MangoHelpe
isDeletable: this.isDeletable(doc),
isEditable: this.isEditable(doc)
};
-
},
- filterOutGeneratedMangoDocs: function (doc) {
- if (doc.get && typeof doc.get === 'function') {
- return doc.get('language') !== 'query';
- }
+ // we need to cache the tableview data and manage the state inside the store as we
+ // are able to select and deselect fields using the dropdown which would then lazyly
+ // repopulate the colelctions on a store emit, which is perfectly fine, however we run
+ // into race conditions with other components which also listen to the same store.
--- End diff --
this is super annoying, the problem here is that
1. action requests data, gets data
2. puts into store, store emits change event
3. the change events leads to components pulling the new data
4. the new components render
5 the data was not processed fully yet, (e.g. the schema calculation)
6 old state is displayed and errors occur, as components depends on data that is calculated after the data arrived in the store
i thought of and decided against: cascading stores updates (store depends on updates from another store) and ended up with caching, however this introduced some complexity for managing that state (you have to make sure to override it properly every time and don't get old data and so on)
not happy with this, bit also not sure regarding a better approach? maybe redux can help here with the transformations/reductions that are separate from the store
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r48768931
--- Diff: app/addons/documents/index-results/stores.js ---
@@ -34,21 +32,30 @@ function (app, FauxtonAPI, ActionTypes, HeaderActionTypes, Documents, MangoHelpe
reset: function () {
this._collection = [];
+ this._filteredCollection = [];
+ this._bulkDeleteDocCollection = new Resources.BulkDeleteDocCollection([], {}),
+
this.clearSelectedItems();
this._isLoading = false;
this._textEmptyIndex = 'No Index Created Yet!';
this._typeOfIndex = 'view';
this._lastQuery = null;
- this._bulkDeleteDocCollection = null;
+
+ this._tableViewEnabledFields = [];
--- End diff --
This var isn't 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.
---
[GitHub] couchdb-fauxton pull request: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-168708081
I'll spent the morning going through this, it's pretty dense. One small thing up front:
- when toggling the include_docs option in the header, the position of the select all checkbox shifts right very quickly while the loading lines appear. Looks like maybe the position of the loading lines is forcing it right. This would be fixed if it's removed altogether when the loading lines are present.
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159929402
update:
- use options object for action
- just show filters when no text is there
- fix filter position for just <4 elements
- footer update (partly, because i have to rip out update_seq which never worked on 2.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.
---
[GitHub] couchdb-fauxton pull request: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-168776980
- `index-results/stores.js`, getLastQuery() not referenced anywhere. Legacy?
Bug:
1. in Table View, change what a column shows to a field that's already shown so you'd see the same field listed twice in two columns.
2. Change the SECOND of the duplicate columns to a different field.
Result: it incorrectly alters the first col, not the one you just changed.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159686231
Nice one Robert.
A few UI remarks - probably for Justin/Dante:
<img width="1412" alt="screen shot 2015-11-25 at 9 50 00 am" src="https://cloud.githubusercontent.com/assets/512116/11404980/d38360c4-9359-11e5-965e-649a81b00b45.png">
- I find the Table (All) / Table (Truncated) quite confusing. I didn't realize it has any relation the include docs checkbox in the Query Options tray, so when selecting one to the other, nothing changed in the page. Hate to bring up this old can of worms again, but moving the "include docs" checkbox right into the page would clarify a great deal. It would allow us to drop that new Table (Truncated) one and just have a single "Table" option, which is nice and clear. Checking/Unchecking that field in the
- The positioning of the "Displaying 1 of 2 fields." text looks wrong... not sure how to improve it, but the spacing looks odd.
- I'd favour putting back the down arrow next to the bulk select component, like in the warehousing section. Seems like that checkbox is just kinda hanging out there in the middle of nowhere. I know the dropdown doesn't technically add anything, but visually it makes it work better - even if we only have the single option ("select/unselect").
- *Filter field*: could we add a default placeholder to that field that'll appear when the user deletes the content? I may be dumb but it's not immediately clear what that does, so as many tips we can give to the user the better. The positioning of the little filter icon should be closer to the text box - or maybe inside, like with [Document ID (search icon)] at the top of the page - see screenshot below. Also, could we make the field a little larger (longer?)
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159740208
Nice, thanks Justin. Regarding this one:
> Include docs could be brought out, but I don't understand why we'd do away with the Table All view.
I think once the Include Docs checkbox is in the page it'd then be a bit redundant. The two Table views in the dropdown right now are very similar, the text in parentheses feels verbose, and they both have the same icon. Seems like it's being doubled up. If it was left there, the only difference would be that one would check the Include Docs checkbox (which would be already in the page) and one that doesn't. To me it feel more succinct and simpler with the single table view and let the page changes occur only when the user checks/unchecks the Include Docs checkbox.
---
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: Table view: add prioritized table vi...
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/580#discussion_r45975720
--- Diff: app/addons/documents/header/header.actions.js ---
@@ -42,9 +42,10 @@ function (app, FauxtonAPI, ActionTypes) {
});
},
- enableTableView: function () {
+ enableTableView: function (displayType) {
FauxtonAPI.dispatch({
- type: ActionTypes.ENABLE_TABLE_VIEW
+ type: ActionTypes.ENABLE_TABLE_VIEW,
+ displayType: displayType
--- End diff --
sounds good
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-169067037
This look good. The only thing I spotted that looks wrong was a - possibly unrelated - layout issue on the Views page. Not sure if this was introduced by this ticket or a previous one & wasn't spotted.
<img width="1390" alt="screen shot 2016-01-05 at 9 06 32 am" src="https://cloud.githubusercontent.com/assets/512116/12121882/ada2bc62-b38c-11e5-8652-986c7580121e.png">
---
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: Table view: add prioritized table vi...
Posted by justin-mcdavid-ibm <gi...@git.apache.org>.
Github user justin-mcdavid-ibm commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-169812572
Looks great, Robert. The only thing that we might tweak is some sort of even distribution across the footer for:
Showing document X of X. (Keep left-justification)
Showing X of X columns.
[] Show all Columns
Documents per page: X (Keep right-justification)
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-160181600
@robertkowalski I think having the options in that dropdown change based on that field might be a bit confusing... (I certainly wouldn't make the connection between that and the include_docs field). I say we have a Meeting Of The Minds early next week and chat about this one in person.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r48768727
--- Diff: app/addons/documents/index-results/index-results.components.react.jsx ---
@@ -259,22 +459,11 @@ function (app, FauxtonAPI, React, Stores, Actions, Components, Documents) {
var mainView = isTableView ? this.getTableStyleView(loadLines) : this.getDocumentStyleView(loadLines);
--- End diff --
LoadLines param isn't used in `getTableStyleView()`
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-169781240
Cool - I'll look at that in a separate ticket.
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159962692
@benkeen do the docs have a rev and id? are they special?
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r48768774
--- Diff: app/addons/documents/index-results/index-results.components.react.jsx ---
@@ -57,15 +67,16 @@ function (app, FauxtonAPI, React, Stores, Actions, Components, Documents) {
},
getRowContents: function (element, rownumber) {
--- End diff --
rownumber should be camel
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-168775230
Overall I found this code super dense. It's not so much PR as the legacy code that it's refactoring. So in brief, I'm okay with it being merged because I don't have any better alternatives to what's here - not without a BIG rewrite - and it's going to be refactored again down the road once we remove all Backbone. ClearlyI the root of the complexity is passing Backbone collections / models around the place which can get altered outside the proper flux flow. Until we stop passing them to the stores we'll need workarounds like the cache you implement here.
As a starting point (and probably outside the scope of this ticket) the BulkDeleteDocCollection and BulkDeleteDoc collection/model looks like prime real estate to remove altogether. Rather than instantiate them + pass it through the whole route object -> action -> store -> component -> action I'd lose them altogether, and rely on the store tracking what needs to be deleted and just pass the relevant info to an action + let it handle the ajax call.
I've noted a few small things in the code which I'll comment on now. Other than those, +1.
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on a diff in the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#discussion_r48768606
--- Diff: app/addons/documents/index-results/index-results.components.react.jsx ---
@@ -101,57 +128,217 @@ function (app, FauxtonAPI, React, Stores, Actions, Components, Documents) {
);
},
+ getAttachmentRow: function (el) {
+ var attachmentCount = Object.keys(el._attachments || {}).length;
+ var paperClip = null;
+ var text = null;
+
+ if (attachmentCount) {
+ text = attachmentCount === 1 ? attachmentCount + ' Attachment' : attachmentCount + ' Attachments';
+ paperClip = (
+ <div><i className="icon fonticon-paperclip"></i> {attachmentCount}</div>
+ );
+ }
+
+ return (
+ <td title={text} className="tableview-el-last">
+ {paperClip}
+ </td>
+ );
+ },
+
+ getCopyButton: function (el) {
+ var text = JSON.stringify(el, null, ' ');
+ return (
+ <td title={text} className="tableview-el-copy">
+ <Clipboard onClipboardClick={this.showCopiedMessage} title={text} text={text}/>
+ </td>
+ );
+ },
+
+ showCopiedMessage: function () {
+ FauxtonAPI.addNotification({
+ msg: 'The Document content has been copied to the clipboard.',
+ type: 'success',
+ clear: true
+ });
+ },
+
render: function () {
var i = this.props.index;
+ var docContent = this.props.el.content;
+ var el = this.props.el;
return (
<tr key={"tableview-content-row-" + i}>
- {this.maybeGetCheckboxCell(i)}
- {this.getRowContents(this.props.data, i)}
+ {this.maybeGetCheckboxCell(el, i)}
+ {this.getCopyButton(docContent)}
+ {this.maybeGetSpecialField(el, i)}
+ {this.getRowContents(el, i)}
+ {this.getAttachmentRow(docContent)}
</tr>
);
}
});
+ var WrappedAutocomplete = React.createClass({
+
+ getInitialState: function () {
+ return {
+ showFilters: false
+ };
+ },
+
+ showFilters: function (state) {
+ this.setState({
+ showFilters: state
+ });
+ },
+
+ render: function () {
+
+ function matchItem (item, value) {
+ return (
+ item.indexOf(value) !== -1
+ );
+ }
+
+ function renderItems (items) {
+ return items.map(function (item) {
+ return item;
+ });
+ }
+
+ var menuStyle = {
+ borderRadius: '3px',
+ boxShadow: '0 2px 12px rgba(0, 0, 0, 0.1)',
+ background: 'rgba(255, 255, 255, 0.9)',
+ padding: '0',
+ fontSize: '90%',
+ position: 'fixed',
+ overflow: 'auto',
+ maxHeight: '50%'
+ };
+
+ return (
+ <div className="table-container-autocomplete">
+ <Autocomplete
+ shouldItemRender={matchItem}
+ initialValue={this.props.selectedField}
+ items={this.props.notSelectedFields}
+ menuStyle={menuStyle}
+ onSelect={function (item) {
+ Actions.changeField({
+ rowName: this.props.selectedField,
+ newSelectedRow: item
+ });
+ this.showFilters(false);
+ }.bind(this)}
+ onChange={function (e, text) {
+ if (!text) {
+ this.showFilters(true);
+ return;
+ }
+ this.showFilters(false);
+ }.bind(this)}
+ getItemValue={function (item) { return item; }}
+ renderItem={function (item, isHighlighted) {
+ var highlight = isHighlighted ? 'table-dropdown-item-highlight ' : '';
+ return (
+ <div
+ className={highlight + 'table-dropdown-item'}
+ key={item}>
+ {item}
+ </div>
+ );
+ }} />
+ {this.state.showFilters ? <i className="icon icon-filter"></i> : null}
+ </div>
+ );
+ }
+ });
+
+
var TableView = React.createClass({
+
getContentRows: function () {
var data = this.props.data.results;
- var schema = this.props.data.schema;
var res = data.map(function (el, i) {
--- End diff --
could just return here and drop `res`
---
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: Table view: add prioritized table vi...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-169413110
@benkeen that error happens on master, too
---
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: Table view: add prioritized table vi...
Posted by benkeen <gi...@git.apache.org>.
Github user benkeen commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/580#issuecomment-159950165
One bug I noticed: I had nothing selected, and clicked the select all checkbox. All rows except three got selected. Not sure why.
Hmm... I still firmly think that there's a UX issue we need to resolve with regard to the "include checkbox" thing. Selecting `Table (All)` then `Table (Truncated)` doesn't show anything different unless that's checked, and it *really* looks like a bug to me. I know it's a pain, Robert, but mind if I stall this ticket until we resolve this with Justin?
---
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.
---