kingctan / oppia-origin

Automatically exported from code.google.com/p/oppia
Apache License 2.0
0 stars 0 forks source link

Code review request #811

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Branch name: experimental-rule-dests

Link to the relevant commit(s): 
https://code.google.com/p/oppia/source/detail?r=dc2360a18357bec462b7add49506f481
3527ad3f&name=experimental-rule-dests

Purpose of code changes on this branch:

Two experimental changes based on release testing feedback for v2.0.0.rc.3:
(1) Arrange the state names in the rule destination selector in the order they 
appear in the state graph, instead of alphabetically.
(2) Move the ⟳ indicator in front of the rule name, not after (based on a 
suggestion received during release testing).

When reviewing my code changes, please focus on:

Please checkout and test the experimental-rule-dests branch on your local 
machine, and then make one selection from each of the following:
  (a) I like change 1  /  I don't like change 1  /  I don't care
  (b) I like change 2  /  I don't like change 2  /  I don't care

Amy, if you have time to get feedback from other users, that would also be 
useful.

After the review, I'll: redo the changes that people like on a new commit, get 
a technical code review, and merge that into develop for the upcoming release.

Thanks!

Original issue reported on code.google.com by s...@seanlip.org on 8 Jun 2015 at 7:41

GoogleCodeExporter commented 9 years ago
(a) I like change 1, great idea.

(b) I like change 2 in principle, but now the options don't look aligned 
because the first option feels 'indented' with the icon in front. How about 
adding forward arrows to the other options?

Original comment by amitdeut...@google.com on 8 Jun 2015 at 3:39

GoogleCodeExporter commented 9 years ago
(a) I like change 1!
(b) Agree with Amit's comments, however, I also notice that the ⟳ indicator 
is so small, I don't know what it means. Is there some other way to denote 
this? Throwing out ideas, maybe [ FIRST STATE ], First State (remain on)?

Original comment by amylat...@google.com on 8 Jun 2015 at 8:02

GoogleCodeExporter commented 9 years ago
(a) I like change 1.
(b) Agree with Amit, also; the misalignment is noticeable. I like Amy's 
suggestions, but also throwing in a possible third:

"REPEAT: First State"

Original comment by bhenn...@google.com on 8 Jun 2015 at 11:57

GoogleCodeExporter commented 9 years ago
An alternative thought for (b):

How about three radio buttons, saying:
- Stay on the current state
- Move to a new state [text box]
- Move to an existing state [drop down]

Original comment by amitdeut...@google.com on 9 Jun 2015 at 12:00

GoogleCodeExporter commented 9 years ago
OK, this is getting a little too complicated for the upcoming release. :-)

I'm going to do a separate commit for (a) and get that merged into develop. 
Will leave (b) as-is for now; Ben will decide on a final implementation in his 
upcoming work on the rule editor.

Original comment by s...@seanlip.org on 9 Jun 2015 at 12:28

GoogleCodeExporter commented 9 years ago
Update: part (1) (together with another fix) has been replicated here and will 
be merged into develop after review:

    https://code.google.com/p/oppia/source/detail?r=328877f19c881fc4e86019f7984eae1eb0ea02e8&name=dest-options

Closing this issue and deleting the experimental branch. Thank you all for the 
feedback! Ben has seen the part (2) responses and will take them into account 
when reworking the rule editor.

Original comment by s...@google.com on 9 Jun 2015 at 1:21