Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

conflict-solving: first pass on revision browser #670

Closed
wants to merge 13 commits into from

Conversation

@robertkowalski
Copy link
Contributor

commented Mar 18, 2016

This adds the first iteration of a revision browsing tool for
conflicts that is able to diff documents and to select
conflicting revs as a winner.

Additional changes:

  • adjust button colors
  • add a helper to create an animal db, which also contains the
    zebra doc, which has a conflict from replication
  • display a small indicator in table view, given a doc has conflicts

Testing instructions:

npm run create:animaldb creates a fresh version of the animaldb
with a conflicting doc, the zebra.

@robertkowalski robertkowalski changed the title Demo [WIP] Demo Mar 18, 2016

@robertkowalski robertkowalski force-pushed the robertkowalski:demo branch 2 times, most recently from c7b42b0 to 1b39fc3 Mar 30, 2016

@robertkowalski robertkowalski force-pushed the robertkowalski:demo branch from 1b39fc3 to f1c765e Apr 7, 2016

@robertkowalski robertkowalski force-pushed the robertkowalski:demo branch 5 times, most recently from 43b652f to 46a7f2f Apr 14, 2016

@robertkowalski robertkowalski changed the title [WIP] Demo conflict-solving: first pass on revision browser Apr 19, 2016

@robertkowalski robertkowalski force-pushed the robertkowalski:demo branch from 46a7f2f to edc89cf Apr 19, 2016

show conflicts / conflict-count in table-view
display a small indicator in table view, given a doc has conflicts

@robertkowalski robertkowalski force-pushed the robertkowalski:demo branch from edc89cf to 86cb4b3 Apr 19, 2016

conflict-solving: first pass on revision browser
This adds the first iteration of a revision browsing tool for
conflicts that is able to diff documents and to select
conflicting revs as a winner.

Additional changes:

 - adjust button colors
 - add a helper to create an animal db, which also contains the
   zebra doc, which has a conflict from replication

Testing instructions:

`npm run create:animaldb` creates a fresh version of the animaldb
with a conflicting doc, the `zebra`.

@robertkowalski robertkowalski force-pushed the robertkowalski:demo branch from 86cb4b3 to 3c85cf2 Apr 19, 2016

robertkowalski added some commits Apr 19, 2016

var conflictCount = Object.keys(el._conflicts || {}).length;
var conflictIndicator = null;
var textConflicts = null;

This comment has been minimized.

Copy link
@robertkowalski

robertkowalski Apr 19, 2016

Author Contributor

note to self: use use const and let

@janl

This comment has been minimized.

Copy link
Member

commented Apr 19, 2016

intro blog post including screenshots and video here: https://github.com/robertkowalski/couch-labs/blob/blog/blog/databases-are-not-boring/databases-are-not-boring.md

this is really sweet.

I can’t comment on the code yet, but I just have a tiny feature request (that isn’t blocking for this PR or anything): it’d be nice if the doc-compare view would still show the differences, kinda like GitHub split view:

meta

but yeah, excellent work, thanks so much!

robertkowalski added some commits Apr 20, 2016

@benkeen

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2016

Hey @robertkowalski, running that script doesn't show a conflict for me, see screenshot. Here's the result of the command:

{ ok: true,
  id: 'zebra',
  rev: '3-f4e6133085f26ad8097d45db1aa92c53' }
{ error: 'not_found', reason: 'Database does not exist.' }
{ error: 'not_found', reason: 'Database does not exist.' }
created :)

screen shot 2016-04-20 at 9 54 43 am

Also - could we make the unselected tab ("Table" | "JSON") look a little different? All grey like that makes it look disabled rather than unselected.

className={'btn save ' + this.props.buttonType}
id={this.props.id}
style={this.props.style}
>

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

Couple of minor things:

  • ID isn't a required prop, so it may end up blank. Better to only add the prop if defined.
  • data-id doesn't have default prop value.

This comment has been minimized.

Copy link
@robertkowalski

robertkowalski Apr 20, 2016

Author Contributor

if data-id is undefined it will not render, it is optional, maybe set default explicitly to null?

This comment has been minimized.

Copy link
@robertkowalski

robertkowalski Apr 20, 2016

Author Contributor

@benkeen do you have a passwort, maybe tester & testerpass defined for your couch? then you have to adjust the script (it uses admin party)

@@ -176,6 +185,11 @@
box-shadow: transparent;
}

.Select .Select-menu {

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

How come capitalized?

This comment has been minimized.

Copy link
@robertkowalski

robertkowalski Apr 20, 2016

Author Contributor

internals of the external component (react-select)

@@ -0,0 +1,73 @@
div#dashboard-content div.revision-wrapper {

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

license comment block at top?

<PanelButton title="Upload Attachment" iconClass="icon-circle-arrow-up" onClick={Actions.showUploadModal} />

{this.state.conflictCount ? <PanelButton
title={`Conflicts (${this.state.conflictCount})`}

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

Nice! I didn't know about the backticks, that's brilliant.

@@ -222,7 +245,7 @@ define([
<div className="panel-section view-attachments-section btn-group">
<button className="panel-button dropdown-toggle btn" data-bypass="true" data-toggle="dropdown" title="View Attachments"
id="view-attachments-menu">
<i className="icon fonticon-picture"></i>
<i className="icon icon-paper-clip"></i>
<span>View Attachments</span>{' '}

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

I'd like to move away from the {' '} workarounds and use CSS for all spacing instead. More control that way + feels a little less hacky.

@@ -253,7 +277,7 @@ define([
var iconClasses = 'icon ' + this.props.iconClass;
return (
<div className="panel-section">
<button className="panel-button upload" title={this.props.title} onClick={this.props.onClick}>
<button className={`panel-button ${this.props.className}`} title={this.props.title} onClick={this.props.onClick}>

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

Maybe a default prop? I'm not sure without testing this that it doesn't output "undefined" or something in the DOM.

}

return (
<div className="revision-split-area" style={{padding: '20px 15px', display: 'flex', color: '#fff'}}>

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

May be better to move this to Less or you'll need to include all the cross-browser rules for flex here (see .display-flex() in assets/less/mixins.less).


const ConflictingRevisionsDropDown = ({options, selected, onRevisionClick, onBackwardClick, onForwardClick}) => {
return (
<div style={{maxWidth: '370px', display: 'flex', marginLeft: '-17px'}}>

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

Same here. Better check this page over in FF + IE.

This comment has been minimized.

Copy link
@robertkowalski

robertkowalski Apr 20, 2016

Author Contributor

aw, good point. as i read this i woudl have had a point for our dev COP today: that i feel unsure about inline styles now, meh, next cop

return;
}

RevActions.chooseLeafs(this.props.ours, next);

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

could this be chooseLeaves() ?

onConfirm: React.PropTypes.func.isRequired,
};

const BackForwardControls = ({onClick, forward, backward}) => {

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

ooooh! :)


return [
{ type: 'back', link: previousPage },
{ name: this.docId + ' > Conflicts', link: '#' }

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

Be nice to style this a little better, either by wrapping the > in a span and toning down the colour a bit, or maybe changing it to a &raquo; & doing the same style. But whatever - low priority, we can change it down the road.

];
},

codeEditor: function (database, doc) {
codeEditor: function (databaseName, docId) {
console.log(databaseName, docId);

This comment has been minimized.

Copy link
@benkeen

benkeen Apr 20, 2016

Contributor

should be removed

This comment has been minimized.

Copy link
@robertkowalski

robertkowalski Apr 20, 2016

Author Contributor

thank you!

@benkeen

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2016

Code looks great, @robertkowalski! Comments above.

@robertkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2016

thank you will fix that tomorrow

@robertkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2016

Also - could we make the unselected tab ("Table" | "JSON") look a little different? All grey like that makes it look disabled rather that unselected.

please discuss with justin @benkeen

@benkeen

This comment has been minimized.

Copy link
Contributor

commented Apr 21, 2016

Looks great. The "Do not show this warning message again" - I checked it, selected a winner, then did the same (new page load) but the modal re-appeared. It's not using local storage?

@benkeen

This comment has been minimized.

Copy link
Contributor

commented Apr 21, 2016

yaaaaay +1 once green + above are all handled.

robertkowalski added some commits Apr 21, 2016

asfgit pushed a commit that referenced this pull request Apr 21, 2016

show conflicts / conflict-count in table-view
display a small indicator in table view, given a doc has conflicts

PR: #670
PR-URL: #670
Reviewed-By: Benjamin Keen <ben.keen@gmail.com>

asfgit pushed a commit that referenced this pull request Apr 21, 2016

conflict-solving: first pass on revision browser
This adds the first iteration of a revision browsing tool for
conflicts that is able to diff documents and to select
conflicting revs as a winner.

Additional changes:

 - adjust button colors
 - add a helper to create an animal db, which also contains the
   zebra doc, which has a conflict from replication

Testing instructions:

`npm run create:animaldb` creates a fresh version of the animaldb
with a conflicting doc, the `zebra`.

PR: #670
PR-URL: #670
Reviewed-By: Benjamin Keen <ben.keen@gmail.com>
@robertkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2016

merged as 319ce99 and 3f46050

@robertkowalski

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2016

thank you!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.