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

Xywh panel #169

Merged
merged 9 commits into from
Sep 5, 2016
Merged

Xywh panel #169

merged 9 commits into from
Sep 5, 2016

Conversation

bramalingam
Copy link
Member

Testing instructions :

  1. Try manually manipulating X,Y Width and Height values in the info panel (both single and multi-select cases).
    Check if the changes get updated on the panels.
  2. Try dragging an image from one page to another and check if the X,Y values are reset to 0,0 at the start of every new page.
  3. Multi-select : Select 2 images from 2 different pages (1 image from page 1 and 1 image from page 2) and change X,Y values. Check if the images get aligned to the appropriate X,Y positions within the page that they reside in. (i.e : image 1 is set to X,Y in page 1 , and image 2 is set to the same X,Y in page 2)

@will-moore

}
else {
console.log(attr);
console.log(value);
Copy link
Member

Choose a reason for hiding this comment

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

The console.logs are probably not necessary, are they?

@dominikl
Copy link
Member

Looks good from my point of view... Functionality works like expected.

@will-moore
Copy link
Member

Only the x input is type='number'. I noticed this because Chrome won't set '-' for a number and you see an error in console:

screen shot 2016-08-31 at 17 11 59

I think x, y, width & height need to be "number" inputs.
It might be nice to also use an empty string instead of "-" for x,y,w,h (but not for the other attrs) so you don't force the browser to catch the error.

var this_json = m.toJSON();
// Format floating point values
_.each(["x", "y", "width", "height"], function(a){
if (this_json[a] != "-") {
Copy link
Member

Choose a reason for hiding this comment

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

I noticed you can remove this if check now, since m.toJSON() won't give you any - values.

@will-moore
Copy link
Member

Need to fix the setting of minus values for x & y when not on page 1 (seems to always jump to the previous page).
Also need to disallow width and height to be less than 1 (negative is bad, probably zero is bad too).
You can set a min='1' when you make these into 'number' inputs, but also have to handle this in JavaScript since users can still type in negative numbers.

@will-moore
Copy link
Member

Working nicely now. No console errors, re-render bug fixed.
Good to merge.

@jburel jburel merged commit 6313c25 into ome:master Sep 5, 2016
@bramalingam bramalingam mentioned this pull request Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants