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 #4635 Make the Advanced feature settings(in creator mode) more intuitive #4695

Merged

seanlip merged 4 commits into oppia:develop from ishucr7:intuitivel

Feb 16, 2018

Contributor

ishucr7 commented Feb 14, 2018

No description provided.

ishucr7 added 2 commits

February 14, 2018 21:15


          Fix Added Make the Advanced feature settings more intuitive

4d3e5f6

fix oppia#4635


          Done

830d005

codecov-io commented Feb 14, 2018 •

edited

Loading

Codecov Report

Merging #4695 into develop will decrease coverage by 0.12%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #4695      +/-   ##
===========================================
- Coverage    44.84%   44.71%   -0.13%     
===========================================
  Files          384      384              
  Lines        23339    23406      +67     
  Branches      3769     3791      +22     
===========================================
  Hits         10467    10467              
- Misses       12872    12939      +67

Impacted Files	Coverage Δ
...s/interactions/GraphInput/directives/GraphInput.js	`31.88% <0%> (-4.08%)`	⬇️
...h_text_components/Link/directives/LinkDirective.js	`5.55% <0%> (-0.33%)`	⬇️
...es/exploration_player/SupplementalCardDirective.js	`2.17% <0%> (-0.21%)`	⬇️
...ditor/editor_tab/CollectionNodeCreatorDirective.js	`1.44% <0%> (-0.05%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 181e8ec...bf4f38c. Read the comment docs.

seanlip reviewed

View reviewed changes

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

@@ @@ -208,8 +214,11 @@ <h3>Advanced Features</h3> @@
                               </span>
                             </button>
                           </span>
-                          <span class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
-                            This allows the user to categorise answer groups as correct or incorrect.
+                          <span ng-if="!isCorrectnessFeedbackEnabled()" class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">

Member

seanlip Feb 14, 2018

@ishucr7 Can you send two screenshots of this (showing the two states) to Joe and get feedback?

Contributor Author

ishucr7 Feb 15, 2018

Sure!

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

+                            Automatic text-to-speech feature is disabled. It generates audio from your content for learners to listen to.
+                          </span>
+                          <span ng-if="isAutomaticTextToSpeechEnabled()" class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
+                            Automatic text-to-speech feature is enabled. It generates audio from your content for learners to listen to. It is recommended that you disable this feature if creating an exploration whose content consists of multiple languages.

Member

seanlip Feb 14, 2018

Automatic text-to-speech is enabled. It generates ...

Contributor Author

ishucr7 Feb 15, 2018

Done!

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

-                          <span class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
-                            Automatic text-to-speech generates audio from your content for learners to listen to. This feature is enabled by default. It is recommended that you disable this feature if creating an exploration whose content consists of multiple languages.
+                          <span ng-if="!isAutomaticTextToSpeechEnabled()" class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
+                            Automatic text-to-speech feature is disabled. It generates audio from your content for learners to listen to.

Member

seanlip Feb 14, 2018

Automatic text-to-speech is disabled. It ...

Contributor Author

ishucr7 Feb 15, 2018

Done!

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

-                          <span class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
-                            Values that change as the learner moves between cards (<a href="http://oppia.github.io/#/Parameters" target="_blank">more info</a>).
+                          <span ng-if="!areParametersEnabled()" class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
+                            Usage of parameters is disabled. Values that change as the learner moves between cards (<a href="http://oppia.github.io/#/Parameters" target="_blank">more info</a>).

Member

seanlip Feb 14, 2018

Usage of parameters is disabled. --> Parameters are disabled.

Values that --> Parameters are values that

Contributor Author

ishucr7 Feb 15, 2018

Done!

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

+                            Usage of parameters is disabled. Values that change as the learner moves between cards (<a href="http://oppia.github.io/#/Parameters" target="_blank">more info</a>).
+                          </span>
+                          <span ng-if="areParametersEnabled()" class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
+                            Usage of parameters is enabled. Values that change as the learner moves between cards (<a href="http://oppia.github.io/#/Parameters" target="_blank">more info</a>).

Member

seanlip Feb 14, 2018

Similar changes suggested to above.

Contributor Author

ishucr7 Feb 15, 2018

Done!

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

@@ @@ -164,8 +164,11 @@ <h3>Advanced Features</h3> @@
                               Enabled
                             </span>
                           </span>
-                          <span class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
-                            Values that change as the learner moves between cards (<a href="http://oppia.github.io/#/Parameters" target="_blank">more info</a>).
+                          <span ng-if="!areParametersEnabled()" class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">

Member

seanlip Feb 14, 2018

Can you move the ng-if one level in, so that you don't need to duplicate the styles?

Also, where possible, put the ng-if only around the "disabled/enabled" sentence. Try not to duplicate the same things across branches. Ditto elsewhere.

Contributor Author

ishucr7 Feb 15, 2018

Done!


          Updated according to the comments

617a794

seanlip reviewed

View reviewed changes

core/templates/dev/head/pages/exploration_editor/settings_tab/settings_tab.html Outdated

@@ @@ -209,6 +219,13 @@ <h3>Advanced Features</h3> @@
                             </button>
                           </span>
                           <span class="col-lg-8 col-md-8 col-sm-8 help-block" style="font-size: smaller;">
+                            The feature is

Member

seanlip Feb 15, 2018

The --> This

Otherwise, PR looks good. I'll wait for you to get confirmation of the screenshots from Joe (as discussed before) before giving LGTM.

Contributor Author

ishucr7 Feb 15, 2018

Okay! I sent the screenshots to Joe, will inform you as soon as I get the feedback.

Contributor Author

ishucr7 Feb 16, 2018

Done!

seanlip added the PR: LGTM label

seanlip approved these changes

View reviewed changes

Member

seanlip left a comment

LGTM

Member

seanlip commented Feb 16, 2018

Oh wait, there's still a small wording change (The --> This). But after that feel free to merge. Thanks!

Contributor Author

ishucr7 commented Feb 16, 2018 via email

Yep! Will correct that in a while.


          changed The -> This

bf4f38c

Member

seanlip commented Feb 16, 2018

All good. Thanks!

seanlip merged commit 71be734 into oppia:develop

code247 pushed a commit to code247/oppia that referenced this pull request


          Fix oppia#4635 Make the Advanced feature settings(in creator mode) mo…

a02e535

…re intuitive (oppia#4695)

* Fix Added Make the Advanced feature settings more intuitive
fix oppia#4635

* Done

* Updated according to the comments

* changed The -> This

code247 pushed a commit to code247/oppia that referenced this pull request


          Fix oppia#4635 Make the Advanced feature settings(in creator mode) mo…

27ee5f7

…re intuitive (oppia#4695)

* Fix Added Make the Advanced feature settings more intuitive
fix oppia#4635

* Done

* Updated according to the comments

* changed The -> This

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels