Xywh width height button #193

bramalingam · 2017-01-24T09:58:42Z

This PR adds an extra button to control the aspect ratio of the panels, when the user edits the width/height of the image.

To Test:

Test if toggling the glyphicon-link button changes status of glyphicon-ok-sign to show/hide.
When the glyphicon-ok-sign is hidden, edit width or height of the image. This will just change the width or height of the image (alone).
When the glyphicon-ok-sign is shown, edit width or height of the image. Please check the width/height ratio (aspect ratio) of the panel before editing. Post editing the aspect ratio should be maintained.
Check if the tooltip on the button makes sense.
And as always code review would be useful as well.
The branch includes the code-layout branch as well, and the PR could technically be considered as an independent review of the code-layout PR(#178) from @will-moore
@will-moore @jburel @aleksandra-tarkowska

Conversation trace from closed PR: #187

… between width and height

…utton Conflicts: omero_figure/static/figure/figure.js omero_figure/static/figure/templates.js

will-moore · 2017-01-24T12:19:51Z

Looks good and is working well.
Minor bug: when the panel is first rendered, you have the green 'check' showing even though the link button is not 'checked'.
You could show/hide the green check purely using the "active" class on the parent button.

  .active .aspect_ratio_selected { display: inline-block }
  .aspect_ratio_selected {
    display: none
    }

Actually it would also be nicer to add the other styles 'color: green' etc to the css .aspect_ratio_selected class instead of in-line styles.

Also, as mentioned before, you need to use m.save(newValues) instead of m.set(newValues) to enable the figure "Save" button.

will-moore · 2017-01-24T12:21:18Z

You need to remove the compiled figure.js and templates.js files. I thought the 'merge' had removed them but it seems not.

will-moore · 2017-01-24T12:23:17Z

src/js/views/right_panel_view.js

-            }, 50);
+                // keep locked status of the aspect ratio button the same,
+                // when the focus shifts because of a blur event
+                if (aspectRatioStatus === true) {


Don't need === true here. Just if (aspectRatioStatus)

will-moore · 2017-01-24T12:24:52Z

src/js/views/right_panel_view.js

+            }
+            if (!$(".set_aspect_ratio", this.$el).hasClass("active")) {
+                $(".aspect_ratio_selected", this.$el).removeClass("glyphicon-ok-sign");
+            }


If you style the . aspect_ratio_selected to only be visible when the parent is .active (see other comment) then you can get rid of all this extra logic.

will-moore · 2017-01-24T12:25:52Z

src/templates/xywh_panel_template.html

-                    </div>
-                    <div class="form-group" style="margin-bottom: -15px;">
+                        </div>                     
+                    </div>                      


The only diff here is adding whitespace. Would be nice to clean that up (as you mentioned).

…n link is not checked

bramalingam · 2017-01-25T17:07:27Z

@will-moore I have pushed two commits. Please check and let me know if it looks fine.

will-moore · 2017-01-26T22:11:14Z

@bramalingam If I check the 'link', change the Width then move focus away (E.g. click on X input) then the panel is re-rendered, the 'link' button is maintained as '.active' but we've lost the green check.
See my previous comments on how to tie the 'green check' to the .active class via css instead of JavaScript, so you don't need to update them both at the same time in JS.

Also, as mentioned before (twice), you need to use m.save(newValues) instead of m.set(newValues) to enable the figure "Save" button.

jburel · 2017-02-02T13:43:57Z

@bramalingam: ready for review?

bramalingam · 2017-02-02T13:54:39Z

Yes it is, guess @will-moore has already reviewed the changes but yeah I can re-list on for review tomorrow.

jburel · 2017-02-03T14:17:36Z

@will-moore could you check the last commits?

will-moore · 2017-02-03T15:29:20Z

src/templates/xywh_panel_template.html

-                            <span class="glyphicon glyphicon-ok-sign aspect_ratio_selected" style="left:187px; top:-51px; font-size:10px; color:green"></span>
-                    </button>
+                    <button type="button" class="btn btn-link setAspectRatio"
+                        style="position:absolute; left:195px; top:25px; padding:3px; width:23px; height:24px; outline:none !important";title="Maintain aspect ratio"><span class="glyphicon glyphicon-link"></span><span class="glyphicon glyphicon-ok-sign okSign" style="left:-28px; top:-7px; font-size:10px; color:green"></span></button>


The button tooltip isn't working because you've got ;title there (remove the ;)

will-moore · 2017-02-03T15:30:44Z

Working nicely now - Just a tiny typo fix for ;title.
Otherwise good to merge.

jburel · 2017-03-28T12:10:24Z

@will-moore happy now with the last commit

will-moore · 2017-03-28T12:20:51Z

Yes, looks good now thanks.

Balaji Ramalingam added 4 commits November 30, 2016 14:52

Add button for controlling aspect ratio and manage callback

c62313e

Manage Button color and add readonly box to highlight the link button…

383b872

… between width and height

Make changes based on Will's suggestions

b81be85

Merge remote-tracking branch 'origin/master' into xywh_width_height_b…

5761db0

…utton Conflicts: omero_figure/static/figure/figure.js omero_figure/static/figure/templates.js

will-moore reviewed Jan 24, 2017

View reviewed changes

Balaji Ramalingam added 2 commits January 25, 2017 17:04

remove complied figure.js and templates.js

20befb7

Remove whitespace, ===true, and hide glyphicon-ok-sign by default whe…

9c1edbd

…n link is not checked

Balaji Ramalingam added 2 commits January 27, 2017 14:16

commit based on feedback

e0a40a1

Clean up and make button names fit to common naming policy

1a3939b

will-moore reviewed Feb 3, 2017

View reviewed changes

Fix tool tip typo

c263aeb

jburel merged commit c00831d into ome:master Mar 28, 2017

jburel added this to the 3.0.0 milestone Jun 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xywh width height button #193

Xywh width height button #193

bramalingam commented Jan 24, 2017

will-moore commented Jan 24, 2017

will-moore commented Jan 24, 2017

will-moore Jan 24, 2017

will-moore Jan 24, 2017

will-moore Jan 24, 2017

bramalingam commented Jan 25, 2017

will-moore commented Jan 26, 2017

jburel commented Feb 2, 2017

bramalingam commented Feb 2, 2017

jburel commented Feb 3, 2017

will-moore Feb 3, 2017

will-moore commented Feb 3, 2017

jburel commented Mar 28, 2017

will-moore commented Mar 28, 2017

Xywh width height button #193

Xywh width height button #193

Conversation

bramalingam commented Jan 24, 2017

will-moore commented Jan 24, 2017

will-moore commented Jan 24, 2017

will-moore Jan 24, 2017

Choose a reason for hiding this comment

will-moore Jan 24, 2017

Choose a reason for hiding this comment

will-moore Jan 24, 2017

Choose a reason for hiding this comment

bramalingam commented Jan 25, 2017

will-moore commented Jan 26, 2017

jburel commented Feb 2, 2017

bramalingam commented Feb 2, 2017

jburel commented Feb 3, 2017

will-moore Feb 3, 2017

Choose a reason for hiding this comment

will-moore commented Feb 3, 2017

jburel commented Mar 28, 2017

will-moore commented Mar 28, 2017