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.

Sign up for GitHub

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

Jump to bottom

Polygon support #8

Merged
merged 7 commits into from
Mar 28, 2018
Merged

Polygon support #8

merged 7 commits into from
Mar 28, 2018

Conversation

will-moore
Copy link
Member

The head of this branch was pushed to npm as version 3.1.0.
This includes all changes currently in ome/omero-figure#253 which loads shape-editor 3.1.0 from npm.

jburel
jburel reviewed Mar 27, 2018
View reviewed changes
src/js/shapeManager.js
@@ -83,12 +85,17 @@ var ShapeManager = function ShapeManager(elementId, width, height, options) {
function(){
self.stopDrag.apply(self, arguments);
});
// this.newShapeBg.click(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be removed

src/js/shapes/polygon.js
this._id = this.manager.getRandomId();
}
this._points = options.points;
// this._rotation = options.rotation || 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be removed

src/js/shapes/polygon.js
this.element.drag(
function(dx, dy) {
// DRAG, update location and redraw
dx = dx / self._zoomFraction;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be 0?

src/js/shapes/polygon.js
var rv = {
'type': "Polygon",
'points': this._points,
// 'rotation': this._rotation,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be removed

src/js/shapes/polygon.js
this.element.attr({'path': path,
'stroke': strokeColor,
'stroke-width': strokeW});
// this.element.transform('r'+ this._rotation);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

src/js/shapes/polygon.js
handle = self.paper.rect(hx-hsize/2, hy-hsize/2, hsize, hsize);
handle.attr({'cursor': 'move'});
handle.h_id = i;
// handle.line = self;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to remove

@jburel
Copy link
Member

jburel commented Mar 28, 2018

You indicate that shape-editor was pushed to npm.
We will need to formalize that workflows similar to what we did for pip
Are you the sole owner of the account?
since pushing to npm will be in that case part of the release process

@will-moore
Copy link
Member Author

Yes - I am the sole owner.

$ npm view ome-shape-editor
 
{ name: 'ome-shape-editor',
  'dist-tags': { latest: '3.1.0' },
  versions: [ '3.1.0' ],
  time: 
   { created: '2018-03-01T13:42:29.322Z',
     '3.1.0': '2018-03-01T13:42:29.389Z',
     modified: '2018-03-01T13:42:29.820Z' },
  maintainers: [ 'will-moore <w.moore@dundee.ac.uk>' ],
...

If you create an account https://docs.npmjs.com/cli/adduser I can add you as an owner https://docs.npmjs.com/cli/owner

@jburel
Copy link
Member

jburel commented Mar 28, 2018 •
edited
Loading

now we have a mismatch the version on npm and no tag on github
so we will need to sort it out. Let's have a chat before we do anything

@will-moore
Copy link
Member Author

As discussed briefly... Any changes to this PR, and merging with develop will make a version 3.1.1 that will need pushing to npm. But, we haven't tagged 3.1.0 on github yet.
Solution is to merge this PR as it is, tag that with 3.1.0.
Then do fixes in another PR, merge and tag with 3.1.1 and push this to npm.
Then we can update OMERO.figure to use that.

@jburel
Copy link
Member

jburel commented Mar 28, 2018

Merging as it is to match publicly available version

@jburel jburel merged commit 6e9e408 into ome:master Mar 28, 2018
@will-moore will-moore mentioned this pull request Mar 29, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jburel jburel jburel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@will-moore @jburel