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

Fix #9134: Fixed bugs with the Drag-and-drop interaction #9141

Merged
merged 7 commits into from
Apr 29, 2020
Merged

Fix #9134: Fixed bugs with the Drag-and-drop interaction #9141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e2f267c
Added validations and changed default value for dnd.
iamprayush Apr 22, 2020
28002fb
Fixed minor bugs.
iamprayush Apr 23, 2020
846dab5
Added frontend test.
iamprayush Apr 23, 2020
385c0fb
Fixed linting errors.
iamprayush Apr 23, 2020
74cf526
Merge branch 'develop' of https://github.com/oppia/oppia into fix-913…
iamprayush Apr 23, 2020
dc8eac8
Merge branch 'develop' of https://github.com/oppia/oppia into fix-913…
iamprayush Apr 25, 2020
fcfc631
Fixed break.
iamprayush Apr 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion ...tions/DragAndDropSortInput/directives/drag-and-drop-sort-input-validation.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('DragAndDropSortInputValidationService', () => {
value: ['Item 1', 'Item 2', 'Item 3']
},
allowMultipleItemsInSamePosition: {
value: false
value: true
}
};

Expand Down Expand Up @@ -130,6 +130,17 @@ describe('DragAndDropSortInputValidationService', () => {
expect(warnings).toEqual([]);
});

it('should not allow multiple items in same position', () => {
customizationArgs.allowMultipleItemsInSamePosition = false;
var warnings = validatorService.getAllWarnings(
currentState, customizationArgs, answerGroups, goodDefaultOutcome);
expect(warnings).toEqual([{
type: WARNING_TYPES.ERROR,
message: 'Multiple items in a single position are not allowed.'
}]);
customizationArgs.allowMultipleItemsInSamePosition = true;
});

it('should expect all items to be nonempty', () => {
// Add rule containing empty items.
answerGroups[0].rules = [equalsListWithEmptyValuesRule];
Expand Down
11 changes: 11 additions & 0 deletions ...teractions/DragAndDropSortInput/directives/drag-and-drop-sort-input-validation.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ export class DragAndDropSortInputValidationService {
for (var j = 0; j < rules.length; j++) {
var inputs = rules[j].inputs;
var rule = rules[j];
if (!customizationArgs.allowMultipleItemsInSamePosition.value) {
for (var i = 0; i < rule.inputs.x.length; i++) {
if (rule.inputs.x[i].length > 1) {
warningsList.push({
type: AppConstants.WARNING_TYPES.ERROR,
message: 'Multiple items in a single position are not allowed.'
});
break;
}
}
}
var range = {
answerGroupIndex: i + 1,
ruleIndex: j + 1
Expand Down
15 changes: 11 additions & 4 deletions extensions/objects/templates/list-of-sets-of-html-strings-editor.directive.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,22 @@
p {
margin-bottom: 10px;
}
.oppia-drag-and-drop-error {
position: absolute;
}
Comment on lines +16 to +18
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, without this, the modal was getting resized every time the error gets rendered.

clunky

I know it's minor, but it was annoying.

</style>

<div class="container-fluid">
<div class="container-fluid" ng-model="$ctrl.localValue"
ui-validate="'$ctrl.validOrdering'" ui-validate-watch="'$ctrl.validOrdering'">
<p>NOTE: Two or more items may occupy the same slot.</p>
<div ng-repeat="choice in $ctrl.choices track by $index">
<div class="row">
<div class="col-sm-1">
<select ng-model="choice.selectedRank" ng-change="$ctrl.selectedItem($index)" aria-label="choice"
id="{{$index}}" ng-options="index for index in $ctrl.allowedChoices()">
<select ng-model="choice.selectedRank"
ng-change="$ctrl.selectItem($index);"
aria-label="choice" id="{{$index}}"
ng-options="index for index in $ctrl.allowedChoices()"
ng-init="choice.selectedRank = $ctrl.initValues[$index]">
</select>
</div>
<div class="col-sm-10">
Expand All @@ -31,6 +38,6 @@
</div>
</div>

<div class="oppia-form-error">
<div class="oppia-form-error oppia-drag-and-drop-error">
<[$ctrl.getWarningText()]>
</div>
98 changes: 55 additions & 43 deletions extensions/objects/templates/list-of-sets-of-html-strings-editor.directive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,93 +30,105 @@ angular.module('oppia').directive('listOfSetsOfHtmlStringsEditor', [
controller: [function() {
var ctrl = this;
var errorMessage = '';
ctrl.validOrdering = true;

ctrl.allowedChoices = function() {
var allowedList = [];
for (var i = 0; i <= Math.min(
ctrl.maxPrevIndex, ctrl.choices.length - 1); i++) {
allowedList.push(i + 1);
for (var i = 1; i <= ctrl.choices.length; i++) {
allowedList.push(i);
}
return allowedList;
};

ctrl.selectedItem = function(choiceListIndex) {
var choiceHtml = ctrl.choices[choiceListIndex].id;
ctrl.selectItem = function(choiceListIndex) {
var choiceId = ctrl.choices[choiceListIndex].id;
var selectedRank = parseInt(
ctrl.choices[choiceListIndex].selectedRank) - 1;
errorMessage = '';
// Reorder the ctrl.choices array to make it consistent with the
// selected rank.
// ctrl.choices.splice(selectedRank, 0, ctrl.choices.splice(
// choiceListIndex, 1)[0]);
var choiceHtmlHasBeenAdded = false;
ctrl.maxPrevIndex = Math.max(selectedRank + 1, ctrl.maxPrevIndex);
var choiceIdHasBeenAdded = false;

for (var i = 0; i < ctrl.value.length; i++) {
choiceHtmlHasBeenAdded = false;
var choiceHtmlIndex = ctrl.value[i].indexOf(choiceHtml);
if (choiceHtmlIndex > -1) {
choiceIdHasBeenAdded = false;
var choiceIdIndex = ctrl.value[i].indexOf(choiceId);
if (choiceIdIndex > -1) {
if (i !== selectedRank) {
ctrl.value[i].splice(choiceHtmlIndex, 1);
ctrl.value[i].splice(choiceIdIndex, 1);
if (ctrl.value[selectedRank] === undefined) {
ctrl.value[selectedRank] = [choiceHtml];
ctrl.value[selectedRank] = [choiceId];
} else {
ctrl.value[selectedRank].push(choiceHtml);
ctrl.value[selectedRank].push(choiceId);
}
}
choiceHtmlHasBeenAdded = true;
choiceIdHasBeenAdded = true;
break;
}
}
for (var i = 0; i < ctrl.value.length; i++) {
if (ctrl.value[i].length === 0) {
if (i === ctrl.value.length - 1) {
// If it is empty list at the last, pop it out.
ctrl.value.pop();
} else {
// Continuity error.
errorMessage = ('No choice(s) is assigned at position ' +
String(i + 1) + '. Please assign some choice at this ' +
'position.');
}
}
}
if (!choiceHtmlHasBeenAdded) {
if (!choiceIdHasBeenAdded) {
if (ctrl.value[selectedRank] === undefined) {
ctrl.value[selectedRank] = [choiceHtml];
ctrl.value[selectedRank] = [choiceId];
} else {
ctrl.value[selectedRank].push(choiceHtml);
ctrl.value[selectedRank].push(choiceId);
}
}
// Removing any empty arrays from the end.
for (var i = 1; i < ctrl.value.length; i++) {
if (ctrl.value[i].length === 0) {
// If empty array is found, all subsequent arrays must also be
// empty since rank skipping is not allowed.
ctrl.value = ctrl.value.slice(0, i);
break;
}
}
ctrl.validateOrdering();
};

ctrl.getWarningText = function() {
return errorMessage;
};
ctrl.$onInit = function() {
if (!ctrl.maxPrevIndex) {
ctrl.maxPrevIndex = 1;

ctrl.validateOrdering = function() {
var selectedRankList = [];
for (var i = 0; i < ctrl.choices.length; i++) {
selectedRankList.push(ctrl.choices[i].selectedRank);
}
selectedRankList.sort();

if (selectedRankList[0] !== 1) {
errorMessage = ('Please assign some choice at position 1.');
ctrl.validOrdering = false;
return;
}
for (var i = 1; i < selectedRankList.length; i++) {
if (selectedRankList[i] - selectedRankList[i - 1] > 1) {
errorMessage = ('Please assign some choice at position ' +
String(selectedRankList[i - 1] + 1) + '.');
ctrl.validOrdering = false;
return;
}
}
errorMessage = '';
ctrl.validOrdering = true;
return;
};

ctrl.$onInit = function() {
ctrl.initValues = [];
ctrl.initArgs = ctrl.getInitArgs();
ctrl.choices = ctrl.initArgs.choices;

// Initialize the default values.
if (ctrl.value[0] === undefined || ctrl.value[0].length === 0) {
ctrl.value = [[]];
ctrl.value = [];
for (var i = 0; i < ctrl.choices.length; i++) {
ctrl.value[0].push(ctrl.choices[i].id);
ctrl.initValues.push(1);
ctrl.choices[i].selectedRank = '';
ctrl.value.push([ctrl.choices[i].id]);
ctrl.initValues.push(i + 1);
}
} else {
for (var i = 0; i < ctrl.choices.length; i++) {
var choice = ctrl.choices[i].id;
ctrl.choices[i].selectedRank = '';
for (var j = 0; j < ctrl.value.length; j++) {
if (ctrl.value[j].indexOf(choice) !== -1) {
ctrl.initValues.push(j + 1);
ctrl.maxPrevIndex = Math.max(ctrl.maxPrevIndex, j + 1);
break;
}
}
Expand Down