Skip to content
This repository has been archived by the owner on Jun 18, 2024. It is now read-only.

Commit

Permalink
Address review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
itsjeyd committed Nov 26, 2015
1 parent d2d0e84 commit 224391a
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 175 deletions.
8 changes: 6 additions & 2 deletions tests/integration/test_vectordraw.py
Original file line number Diff line number Diff line change
Expand Up @@ -712,10 +712,10 @@ def test_reset(self, params):
# Board should not show point initially
points = json.loads(params["points"])
self.check_points(board, points)
# Add point (need to do this first since point is selected initially)
self.add_point(board, points)
# Add vector
self.add_vector(board, vectors)
# Add point
self.add_point(board, points)
# Reset
self.reset(board, vectors, points)

Expand Down Expand Up @@ -1003,3 +1003,7 @@ def test_change_property_invalid_input(self, params):
vectors[0]["expected_tail_position"] = {'cx': 347, 'cy': 181}
vectors[0]["expected_tip_position"] = {'cx': 411, 'cy': 117}
self.check_vectors(board, vectors)
# Check error message
error_message = self.exercise.find_element_by_css_selector(".update-error");
self.wait_until_visible(error_message)
self.assertEquals(error_message.text, "Invalid input.")
39 changes: 22 additions & 17 deletions vectordraw/public/css/vectordraw.css
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,23 @@
width: 100%;
}

.vectordraw_block .menu,
.vectordraw_block .menu .controls {
border-top-left-radius: 5px;
border-top-right-radius: 5px;
background-color: #e0e0e0;
font-size: 0;
}

.vectordraw_block .menu {
border-top: 2px solid #1f628d;
border-left: 2px solid #1f628d;
border-right: 2px solid #1f628d;
}

.vectordraw_block .menu .controls {
border-bottom: 2px solid #1f628d;
padding: 3px;
background-color: #e0e0e0;
font-size: 0;
}

.vectordraw_block .menu .controls select {
Expand Down Expand Up @@ -72,10 +80,6 @@
}

.vectordraw_block .menu .vector-properties {
border-top: 2px solid #1f628d;
border-left: 2px solid #1f628d;
border-right: 2px solid #1f628d;
border-bottom: 0px none;
padding: 10px;
font-size: 16px;
line-height: 1.25;
Expand All @@ -96,16 +100,7 @@
display: table-row;
}

.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-name,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-tail,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-tail-label,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-length,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-length-label,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-angle,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-angle-label,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-slope,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-slope-label,
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-update {
.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop {
display: table-cell;
width: 50%
}
Expand All @@ -116,12 +111,22 @@
padding-right: 5px;
}

.vectordraw_block .menu .vector-properties .vector-prop-list .row span,
.vectordraw_block .menu .vector-properties .vector-prop-list .row select,
.vectordraw_block .menu .vector-properties .vector-prop-list .row input {
float: right;
width: 50%;
}

.vectordraw_block .menu .vector-properties .vector-prop-list .row select,
.vectordraw_block .menu .vector-properties .vector-prop-list .row input {
float: right;
}

.vectordraw_block .menu .vector-properties .vector-prop-list .row .vector-prop-update .update-error {
visibility: hidden;
color: #ff0000;
}

.vectordraw_block .action button {
height: 40px;
margin-right: 10px;
Expand Down
105 changes: 1 addition & 104 deletions vectordraw/public/css/vectordraw_edit.css
Original file line number Diff line number Diff line change
Expand Up @@ -14,116 +14,13 @@
font-size: 0.9em;
}

.vectordraw_edit_block #vectordraw {
margin-bottom: 1.5em;
}

.vectordraw_edit_block .jxgboard {
border: 2px solid #1f628d;
float: none;
margin-bottom: 1em;
}

.vectordraw_edit_block .jxgboard .JXGtext {
pointer-events: none; /* prevents cursor from turning into caret when over a label */
}

/* Menu */

.vectordraw_edit_block .menu {
width: 100%;
}

.vectordraw_edit_block .menu .controls {
border-top-left-radius: 5px;
border-top-right-radius: 5px;
border-top: 2px solid #1f628d;
border-left: 2px solid #1f628d;
border-right: 2px solid #1f628d;
padding: 3px;
background-color: #e0e0e0;
font-size: 0;
}

.vectordraw_edit_block .menu .controls select {
width: 160px;
margin-right: 3px;
font-size: 18px;
}

.vectordraw_edit_block .menu button {
border: 1px solid #1f628d;
border-radius: 5px;
margin: 4px 0;
padding: 5px 10px 5px 10px;
box-shadow: 0 1px 3px #666;
background-color: #c2e0f4;
color: #1f628d;
font-size: 14px;
text-decoration: none;
}

.vectordraw_edit_block .menu button:hover {
background: #c2e0f4;
background-image: -webkit-linear-gradient(top, #c2e0f4, #add5f0);
background-image: -moz-linear-gradient(top, #c2e0f4, #add5f0);
background-image: -ms-linear-gradient(top, #c2e0f4, #add5f0);
background-image: -o-linear-gradient(top, #c2e0f4, #add5f0);
background-image: linear-gradient(to bottom, #c2e0f4, #add5f0);
text-decoration: none;
}

.vectordraw_edit_block .menu .vector-properties {
border-top: 2px solid #1f628d;
border-left: 2px solid #1f628d;
border-right: 2px solid #1f628d;
border-bottom: 0px none;
padding: 10px;
font-size: 16px;
line-height: 1.25;
background-color: #f7f7f7;
}

.vectordraw_edit_block .menu .vector-properties h3 {
font-size: 16px;
margin: 0 0 5px;
}

.vectordraw_edit_block .menu .vector-properties .vector-prop-list {
display: table;
width: 100%
}

.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row {
display: table-row;
}

.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-name,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-tail,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-tail-label,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-length,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-length-label,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-angle,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-angle-label,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-slope,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-slope-label,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-update,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-remove {
display: table-cell;
width: 50%
}

.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-name,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-tail,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-prop-angle {
padding-right: 5px;
}

.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row select,
.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row input {
float: right;
width: 50%;
}

.vectordraw_edit_block .menu .vector-properties .vector-prop-list .row .vector-remove {
vertical-align: bottom;
}
Expand Down
45 changes: 18 additions & 27 deletions vectordraw/public/js/vectordraw.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ function VectorDrawXBlock(runtime, element, init_args) {
}
}

var noAddOptionSelected = true;

function renderAndSetMenuOptions(element, idx, type, board) {
if (element.render) {
if (type === 'point') {
Expand All @@ -82,9 +80,8 @@ function VectorDrawXBlock(runtime, element, init_args) {
var addOption = board.getAddMenuOption(type, idx);
addOption.prop('disabled', false);
// ... and select it if no option is currently selected
if (noAddOptionSelected) {
if ($('.menu .element-list-add option').filter(':selected').length === 0) {
addOption.prop('selected', true);
noAddOptionSelected = false;
}
// Disable corresponding option in menu for editing vectors
var editOption = board.getEditMenuOption(type, idx);
Expand Down Expand Up @@ -473,14 +470,15 @@ function VectorDrawXBlock(runtime, element, init_args) {
newLength = $('.vector-prop-length input', element).val(),
newAngle = $('.vector-prop-angle input', element).val();
// Process values
newTail = _.map(newTail.split(', '), function(coord) {
newTail = _.map(newTail.split(/ *, */), function(coord) {
return parseFloat(coord);
});
newLength = parseFloat(newLength);
newAngle = parseFloat(newAngle);
var values = [newTail[0], newTail[1], newLength, newAngle];
// Validate values
if (!_.some(values, Number.isNaN)) {
$('.vector-prop-update .update-error', element).css('visibility', 'hidden');
// Use coordinates of new tail, new length, new angle to calculate new position of tip
var radians = newAngle * Math.PI / 180;
var newTip = [
Expand All @@ -492,6 +490,8 @@ function VectorDrawXBlock(runtime, element, init_args) {
board_object.point1.setPosition(JXG.COORDS_BY_USER, newTail);
board_object.point2.setPosition(JXG.COORDS_BY_USER, newTip);
this.board.update();
} else {
$('.vector-prop-update .update-error', element).css('visibility', 'visible');
}
};

Expand Down Expand Up @@ -606,31 +606,22 @@ function VectorDrawXBlock(runtime, element, init_args) {
.success(updateStatus);
}

// Logic for dealing with rendering issues

function reRender() {
JXG.JSXGraph.freeBoard(vectordraw.board);
vectordraw.render();
}

// Initialization logic

// Initialize exercise
var vectordraw = new VectorDraw('vectordraw', init_args.settings);
// Initialize exercise.
// Defer it so that we don't try to initialize it for a hidden element (in Studio);
// JSXGraph has problems rendering a board if the containing element is hidden.
window.setTimeout(function() {
var vectordraw = new VectorDraw('vectordraw', init_args.settings);

// Check if board was initialized successfully;
// if not, re-render the exercise:
if (vectordraw.board.canvasWidth === 0 && vectordraw.board.canvasHeight === 0) {
window.setTimeout(reRender, 1);
}

// Load user state
if (!_.isEmpty(init_args.user_state)) {
vectordraw.setState(init_args.user_state);
updateStatus(init_args.user_state);
}
// Load user state
if (!_.isEmpty(init_args.user_state)) {
vectordraw.setState(init_args.user_state);
updateStatus(init_args.user_state);
}

// Set up click handlers
$('.action .check', element).on('click', function(e) { checkAnswer(vectordraw); });
// Set up click handlers
$('.action .check', element).on('click', function(e) { checkAnswer(vectordraw); });
}, 0);

}
5 changes: 4 additions & 1 deletion vectordraw/public/js/vectordraw_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,14 +415,15 @@ function VectorDrawXBlockEdit(runtime, element, init_args) {
newLength = $('.vector-prop-length input', element).val(),
newAngle = $('.vector-prop-angle input', element).val();
// Process values
newTail = _.map(newTail.split(', '), function(coord) {
newTail = _.map(newTail.split(/ *, */), function(coord) {
return parseFloat(coord);
});
newLength = parseFloat(newLength);
newAngle = parseFloat(newAngle);
var values = [newTail[0], newTail[1], newLength, newAngle];
// Validate values
if (!_.some(values, Number.isNaN)) {
$('.vector-prop-update .update-error', element).css('visibility', 'hidden');
// Use coordinates of new tail, new length, new angle to calculate new position of tip
var radians = newAngle * Math.PI / 180;
var newTip = [
Expand All @@ -434,6 +435,8 @@ function VectorDrawXBlockEdit(runtime, element, init_args) {
board_object.point1.setPosition(JXG.COORDS_BY_USER, newTail);
board_object.point2.setPosition(JXG.COORDS_BY_USER, newTip);
this.board.update();
} else {
$('.vector-prop-update .update-error', element).css('visibility', 'visible');
}
};

Expand Down
15 changes: 8 additions & 7 deletions vectordraw/templates/html/vectordraw.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ <h2>{{ self.display_name }}</h2>
{% endif %}

<div id="vectordraw">
<div class="menu" style="width: {{ self.menu_width }}px;">
<div class="menu" style="width: {{ self.width }}px;">
<div class="controls">
<span class="sr" id="element-list-add-label">{% trans "Select element to add to board" %}</span>
<select class="element-list-add" aria-labelledby="element-list-add-label">
Expand Down Expand Up @@ -48,7 +48,7 @@ <h2>{{ self.display_name }}</h2>
<h3>{{ self.vector_properties_label }}</h3>
<div class="vector-prop-list">
<div class="row">
<div class="vector-prop-name">
<div class="vector-prop vector-prop-name">
<span id="vector-prop-name-label">
{% trans "name" %}:
</span>
Expand All @@ -70,39 +70,40 @@ <h3>{{ self.vector_properties_label }}</h3>
</div>
</div>
<div class="row">
<div class="vector-prop-tail">
<div class="vector-prop vector-prop-tail">
<span id="vector-prop-tail-label">
{% trans "tail position" %}:
</span>
<input type="text" value="-" aria-labelledby="vector-prop-tail-label">
</div>
<div class="vector-prop-length">
<div class="vector-prop vector-prop-length">
<span id="vector-prop-length-label">
{% trans "length" %}:
</span>
<input type="text" value="-" aria-labelledby="vector-prop-length-label">
</div>
</div>
<div class="row">
<div class="vector-prop-angle">
<div class="vector-prop vector-prop-angle">
<span id="vector-prop-angle-label">
{% trans "angle" %}:
</span>
<input type="text" value="-" aria-labelledby="vector-prop-angle-label">
</div>
<div class="vector-prop-slope">
<div class="vector-prop vector-prop-slope">
<span id="vector-prop-slope-label">
{% trans "slope" %}:
</span>
<input type="text" value="-" aria-labelledby="vector-prop-slope-label" disabled="disabled">
</div>
</div>
<div class="row">
<div class="vector-prop-update">
<div class="vector-prop vector-prop-update">
<button class="update">
<span class="update-label" aria-hidden="true">{% trans "Update" %}</span>
<span class="sr">{% trans "Update properties of selected element" %}</span>
</button>
<span class="update-error">{% trans "Invalid input." %}</span>
</div>
</div>
</div>
Expand Down
Loading

1 comment on commit 224391a

@bradenmacdonald
Copy link
Member

Choose a reason for hiding this comment

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

@itsjeyd Thanks for these fixes! +1 from me on the accessibility commits now.

Please sign in to comment.