Fix scribing scaling #366

kxmbrian · 2015-01-17T17:04:11Z

Fix for #336. Scribbles should now display consistently on different browser / screen / canvas sizes.

Other minor improvements also included.

houndci-bot · 2015-01-17T17:04:15Z

app/assets/javascripts/scribing_canvas.js

@@ -82,7 +113,7 @@ function loadScribble(c, scribble, layersList) {
      .attr('selected','selected');
    layersList.append(newLayerEntry);

-    var toggleLayer = function (showLayer) {
+    function toggleLayer(showLayer) {


Function declarations should not be placed in blocks. Use a function expression or move the statement to the top of the outer function.

coveralls · 2015-01-17T17:08:24Z

Coverage remained the same when pulling 4cf0998 on kxmbrian:fix-scribing-scaling into 305e5e8 on Coursemology:development.

coveralls · 2015-01-17T17:12:45Z

Coverage remained the same when pulling c295033 on kxmbrian:fix-scribing-scaling into 305e5e8 on Coursemology:development.

allenwq · 2015-01-18T10:18:45Z

@kxmbrian I occasionally found this: https://github.com/Coursemology/coursemology.org/blob/development/app/models/assessment/submission.rb#L23

I think it's a typo ?

kxmbrian · 2015-01-18T16:14:58Z

Good catch. It seems like neither general_answers or scribing_answers are invoked anywhere. Should I be commenting / removing the unnecessary code actually?

coveralls · 2015-01-18T16:16:32Z

Coverage remained the same when pulling af779f9 on kxmbrian:fix-scribing-scaling into 305e5e8 on Coursemology:development.

allenwq · 2015-01-19T11:36:25Z

Yah. seems no where called general_answers so nothing was broken. But better fix it in case someone called it in future.

coveralls · 2015-01-20T12:26:46Z

Coverage decreased (-0.01%) when pulling 4a6eaf3 on kxmbrian:fix-scribing-scaling into 305e5e8 on Coursemology:development.

…icked.

…m is bad.

houndci-bot · 2015-01-22T09:07:19Z

app/assets/javascripts/scribing_canvas.js

+  if (newJSON !== oldJSON) {
+    ajaxField.data('content', newJSON);
+    $.ajax({
+        type: "POST",


Mixed double and single quotes.

…d + tidy up.

houndci-bot · 2015-01-23T01:54:11Z

app/assets/javascripts/scribing_canvas.js

+  //adapted from
+  //http://stackoverflow.com/questions/19682706/how-do-you-close-the-iris-colour-picker-when-you-click-away-from-it
+  $(document).click(function(e) {
+    if (!$(e.target).is('.scribing-color-val .iris-picker .iris-picker-inner')) {


Line is too long.

houndci-bot · 2015-01-23T10:07:02Z

app/assets/javascripts/scribing_canvas.js

+          var newLeft = viewportLeft + deltaLeft;
+          var newTop = viewportTop + deltaTop;
+          tryPan(newLeft, newTop);
+        } else if (options['isForced']) {


['isForced'] is better written in dot notation.

coveralls · 2015-01-23T10:11:19Z

Coverage decreased (-0.01%) to 56.78% when pulling cf73150 on kxmbrian:fix-scribing-scaling into 305e5e8 on Coursemology:development.

…g into fix-scribing-scaling

coveralls · 2015-01-24T09:22:40Z

Coverage remained the same at 56.78% when pulling cc5e173 on kxmbrian:fix-scribing-scaling into 78315e8 on Coursemology:development.

kxmbrian · 2015-01-24T09:43:31Z

@wangqiang1208, this feature is currently frozen and ready for a final check before merge. Could you help to review?

jsyeo · 2015-01-25T19:35:51Z

@wangqiang1208, good to merge?

allenwq · 2015-01-26T02:25:14Z

@kxmbrian @jsyeo Give me a while, I'm testing this !

allenwq · 2015-01-26T03:06:28Z

app/views/assessment/mission_submissions/_scribing_canvas.html.erb

-          <input type="text" class="scribing-color-val" id="scribing-color-<%= qid %>"></input>
-			    <input type="range" id="scribing-width-<%= qid %>" value="2" min="0" max="30" style="width: 100px"></input>
+					<div class="panel">
+	          <input type="text" class="scribing-color-val" id="scribing-color-<%= qid %>"></input>


Not aligned here.

@jsyeo it was here

allenwq · 2015-01-26T03:19:23Z

@kxmbrian @jsyeo I've tested this PR, Tests on Safari and Chrome shows that #336 is fixed.

I haven't looked at javascript code. And this PR seems only changed the scribing related stuff, so should not have side effects on other assessment parts.

Except some minor issues, I think this is good to merge.

coveralls · 2015-01-26T03:48:04Z

Coverage remained the same at 56.78% when pulling b81480b on kxmbrian:fix-scribing-scaling into 78315e8 on Coursemology:development.

jsyeo · 2015-01-26T03:57:11Z

@wangqiang1208 minor issues?

allenwq · 2015-01-27T03:02:27Z

@jsyeo Sorry, didn't notice your message, this can be merged.

jsyeo · 2015-01-27T16:03:30Z

Except some minor issues, I think this is good to merge.

What are the minor issues?

allenwq · 2015-01-27T16:13:02Z

See my comments, actually just line alignments.

On Wednesday, January 28, 2015, Jason Yeo [email protected] wrote:

Except some minor issues, I think this is good to merge.

What are the minor issues?

—
Reply to this email directly or view it on GitHub
#366 (comment)
.

jsyeo · 2015-01-27T16:14:35Z

See my comments

Which one?

allenwq · 2015-01-27T16:15:51Z

Cannot find the last outdated one ?

On Wednesday, January 28, 2015, Jason Yeo [email protected] wrote:

See my comments

Which one?

—
Reply to this email directly or view it on GitHub
#366 (comment)
.

jsyeo · 2015-01-27T16:27:44Z

@kxmbrian please split your PR next time. You are trying to fix 2-3 issues here. A bit hard for me and @wangqiang1208 to review. Thanks. 😄

Fix scribing scaling

kxmbrian · 2015-01-28T01:52:01Z

@jsyeo What should I do if one PR is built on a previous PR that has not been merged?

jsyeo · 2015-01-28T02:41:39Z

@kxmbrian just have to let us know that the PR depends on another PR. (e.g. Coursemology/coursemology2#78 (comment)) I will merge the one that is depended on first.

kxmbrian · 2015-01-28T02:47:56Z

ok thanks 👍

kxmbrian added 3 commits January 18, 2015 00:27

Add (de)normalisation to scribble saving/display

13a7f15

Increase allowable scribble length

5b40bf6

Prevent ajax call when scribble not changed

4cf0998

houndci-bot reviewed Jan 17, 2015
View reviewed changes

Tidy up as per hound recommendation

c295033

Fix typo

af779f9

Improve aesthetic

4a6eaf3

kxmbrian added 4 commits January 21, 2015 18:53

Change icons

61272e1

Fix panning bug. Panning no longer starts as soon as the button is cl…

c452f31

…icked.

Disallow editing scribing answer by student after submission

9bf53e7

Used jquery for ajax call instead of form submission. Form within for…

5d6d934

…m is bad.

houndci-bot reviewed Jan 22, 2015
View reviewed changes

Prevent attempt to hide iris colour picker when picker not initialise…

68db231

…d + tidy up.

houndci-bot reviewed Jan 23, 2015
View reviewed changes

kxmbrian added 2 commits January 23, 2015 15:36

Prevent border from showing when zooming out at corners

dea41f0

Disable all scribble selection when in grab mode

cf73150

houndci-bot reviewed Jan 23, 2015
View reviewed changes

Merge branch 'development' of github.com:Coursemology/coursemology.or…

cc5e173

…g into fix-scribing-scaling

allenwq reviewed Jan 26, 2015
View reviewed changes

Improve formatting

b81480b

jsyeo added a commit that referenced this pull request Jan 27, 2015

Merge pull request #366 from kxmbrian/fix-scribing-scaling

9edc5fc

Fix scribing scaling

jsyeo merged commit 9edc5fc into Coursemology:development Jan 27, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix scribing scaling #366

Fix scribing scaling #366

kxmbrian commented Jan 17, 2015

houndci-bot Jan 17, 2015

coveralls commented Jan 17, 2015

coveralls commented Jan 17, 2015

allenwq commented Jan 18, 2015

kxmbrian commented Jan 18, 2015

coveralls commented Jan 18, 2015

allenwq commented Jan 19, 2015

coveralls commented Jan 20, 2015

houndci-bot Jan 22, 2015

houndci-bot Jan 23, 2015

houndci-bot Jan 23, 2015

coveralls commented Jan 23, 2015

coveralls commented Jan 24, 2015

kxmbrian commented Jan 24, 2015

jsyeo commented Jan 25, 2015

allenwq commented Jan 26, 2015

allenwq Jan 26, 2015

allenwq Jan 27, 2015

allenwq commented Jan 26, 2015

coveralls commented Jan 26, 2015

jsyeo commented Jan 26, 2015

allenwq commented Jan 27, 2015

jsyeo commented Jan 27, 2015

allenwq commented Jan 27, 2015

jsyeo commented Jan 27, 2015

allenwq commented Jan 27, 2015

jsyeo commented Jan 27, 2015

kxmbrian commented Jan 28, 2015

jsyeo commented Jan 28, 2015

kxmbrian commented Jan 28, 2015

Fix scribing scaling #366

Fix scribing scaling #366

Conversation

kxmbrian commented Jan 17, 2015

houndci-bot Jan 17, 2015

Choose a reason for hiding this comment

coveralls commented Jan 17, 2015

coveralls commented Jan 17, 2015

allenwq commented Jan 18, 2015

kxmbrian commented Jan 18, 2015

coveralls commented Jan 18, 2015

allenwq commented Jan 19, 2015

coveralls commented Jan 20, 2015

houndci-bot Jan 22, 2015

Choose a reason for hiding this comment

houndci-bot Jan 23, 2015

Choose a reason for hiding this comment

houndci-bot Jan 23, 2015

Choose a reason for hiding this comment

coveralls commented Jan 23, 2015

coveralls commented Jan 24, 2015

kxmbrian commented Jan 24, 2015

jsyeo commented Jan 25, 2015

allenwq commented Jan 26, 2015

allenwq Jan 26, 2015

Choose a reason for hiding this comment

allenwq Jan 27, 2015

Choose a reason for hiding this comment

allenwq commented Jan 26, 2015

coveralls commented Jan 26, 2015

jsyeo commented Jan 26, 2015

allenwq commented Jan 27, 2015

jsyeo commented Jan 27, 2015

allenwq commented Jan 27, 2015

jsyeo commented Jan 27, 2015

allenwq commented Jan 27, 2015

jsyeo commented Jan 27, 2015

kxmbrian commented Jan 28, 2015

jsyeo commented Jan 28, 2015

kxmbrian commented Jan 28, 2015