docs(architecture): proofread, updated Dart app#1807
docs(architecture): proofread, updated Dart app#1807chalin wants to merge 4 commits intoangular:masterfrom
Conversation
|
@kwalrath @thso : Dart-side commit ready for review. |
| header Dart difference: Module is a compilation unit | ||
| :marked | ||
| The term "module" used in this guide refers to a | ||
| Dart _compilation unit_, i.e., a _library_ or _part_. |
There was a problem hiding this comment.
i.e. -> that is
(many people don't know what i.e. means)
Actually, though... do we really need to mention parts? They're strongly discouraged. Also, people might not know that each file without a library directive is itself a library. Perhaps instead:
In this guide, the term module refers to a Dart compilation unit, such as a library. If a Dart file has no library or part directive, then that file itself is a library and thus a compilation unit. For more information...
There was a problem hiding this comment.
Fixed as you suggested with tweaks we agree to elsewhere (in particular clarifying that a Dart package is also a module).
|
Looking good! Once you address a few comments, the Dart side is good to go. |
chalin
force-pushed
the
chalin-architecture-review-0630
branch
from
5488fec to
f20221c
Compare
|
Dart-side issues addressed. |
|
LGTM! @Foxandxss or @wardbell, who's going to review the TS side? |
chalin
force-pushed
the
chalin-architecture-review-0630
branch
3 times, most recently
from
70f6eeb to
a59aafe
Compare
| selector: 'sales-tax', | ||
| template: ''' | ||
| <h2>Sales Tax Calculator</h2> | ||
| Amount: <input #amountBox (input)="0" type="number"> |
There was a problem hiding this comment.
What triggered this change (also in TS)? I am not sure I like (input) as the event name. I don't know, I guess it is just me but I liked (change) more. Input is also the element name so as an event again is weird. Also input sounds more like an input (pun intended) than as an output.
On the other hand, the number input is really broken in other browsers. I don't think they are fixed yet.
There was a problem hiding this comment.
Triggering on onInput rather than onChange makes for a more responsive user experience (UX). Again, using type="number" makes for a better UX in those browsers that support it, and makes no difference (from the app processing point of view) in those browsers that do not support it.
There was a problem hiding this comment.
Please revert to (change). We're not trying to be responsive. We're trying to make a point about how (someEvent)="0" is a trick to cause dirty checking when the user enters something. (input) invites the kind of distracting question that Jesus asked.
Please revert type="number". This is a distraction from the point. Again, we're not looking to produce the best app solution here. We're trying to make a narrow point. Let's not add distractions from that point.
5 participants
Note that there are two commits: one for each of TS- and Dart-side changes.
docregions).Contributes to #1598 and #1508.