Closed Janamou closed 9 years ago
Hi, amazing work. CIL below. What I don't comment on explicitly, I agree on. When I agree there's a bug, please put it in pivotal so we have one canonical place to track work. Thanks a zillion!
The button is never set to disabled in HTML. This is correct behavior?
That definitely seems like a bug. Well spotted.
When I then test FormProxy it doesn’t have these attributes set and the test fails:
Same here, also a bug.
When I do this test, the commented part (because it fails) and the second print statement returns text without html. Is this correct behavior? E.g. instead of
Initial text.
it returns Initial text.:
I think that's a bug, but please put it as low priority. I think that users shouldn't actually have access to uiElement.html (as it stands behind proxy). But I'll dig into this.
Competer.complete missing
Absolutely. File a bug or feel free to fix if it's a small one and you'd like to.
EgbChoice toMapForPresenter() doesn’t save also goto and other attributes to the map. Is it correct?
That's correct, at least for goto
. Presenter shouldn't worry about the value of goto
(it doesn't display it) and, in fact, it's better if it doesn't know. For example, it would then be easier to "hack" the game by checking goto
attributes of every choice (these are page names, and the author can have internal info in them — something like "Part 3: Death").
There might be a better (type-safe) way to do this (like an intermediate class?) but I don't want to increase complexity of the code by yet another proxy class. If I'm missing some obvious solution, please let me know.
Failing test because of Future already completed
I'll look into it. Can you please file a bug in pivotal against me? Just point me to the file. I don't know when I'll be able to get to it.
Comments/needs to be fixed in order to run the tests.
FormElement
I changed the
[:false:]
to[:true:]
in the comment - because its hidden.FormSection
Why is this class not named
FormSectionBase
? Comparing to the functionality in other classes, it has similar functionality as “Base" classes. And for exampleTextOutput
is empty class becauseTextOutputBase
exists. Should not be forFormSection
something similar?README
I updated README with the tool/tester. Can I also mention running through the egamebook.dart and running doc via docgen script?
Tests
Disabled
When I do with test this:
The button is never set to
disabled
in HTML. This is correct behavior? The reason is that when you are converting from JSONML in FormProxy.fromMap, not all attributes are set (likedisabled
,hidden
etc.).Disabled in FormProxy
When I then test
FormProxy
it doesn’t have these attributes set and the test fails:Stripping of HTML tags
When I do this test, the commented part (because it fails) and the second
print
statement returns text without html. Is this correct behavior? E.g. instead of<p>Initial text.</p>
it returnsInitial text.
:Competer.complete missing
Method
showChoices()
never callscompleter.complete()
and this is the reason why the async tests will not work (because of problem with timeout - changing timeout doesn’t work) - it has to call completer.complete.Similar problem with
showDialog()
- it hascompeter.complete()
only in condition so the test expires.EgbChoice toMapForPresenter()
doesn’t save also
goto
and other attributes to the map. Is it correct?Failing test because of Future already completed
I can’t make work commented part of “Show simple dummy choices” test - when the
click()
on button is called, the test fails with “Future already completed“. Do you have idea where could be problem?