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.

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

Round 2 codelab review #3

Open
jmandel opened this issue Dec 11, 2019 · 0 comments
Open

Round 2 codelab review #3

jmandel opened this issue Dec 11, 2019 · 0 comments
Assignees

Comments

@jmandel
Copy link
Member

jmandel commented Dec 11, 2019

Example App

I love how the intro is evolving; "Example App" intro is clear and glitch embedding works. In the embedded demo, it'd be good to pre-fill a username and password just to save typing / simplify the explanation (but preserving the fact that a click-through is required, to show the workflow pain). Given that glitch embedding presents an iframe with no browser controls, might also want to add a "Back" button in the embedded demo, or better still, show the result on the same page as the form so no nav is required.

Language in example cards: "This order is not specified by AUC guidelines" could be something more aligned with the CMS terminology (e.g., "No AUC guidelines apply to this type of order" if that's what is being represented under the hood).

Exercise Outline

Really nice context here. Can you confirm you're still planning to support systemActions and mention this in the outline (assuming the answer is yes)?

The "final product" screenshot might logically be better to show in the CDS Hooks sandbox, since that's where people will run it. But I understand that the behavior of popping out into a new window means there's no graphical embedding to show. I don't have a clear recommendation here :)

On FAQ, I think the forthright tone is great here. Rather than calling out vendors by name though, it strikes me that docs like this tutorial have a way of lasting longer than anyone expects -- so just saying ~"it's early, and there's a great opportunity to join this community" and linking out to the readiness matrix (perhaps augmenting the readiness matrix to describe PAMA support) would be great.

Enable SMART Launch

For the "spoiler" solution, probably worth showing the npm install command (including target version of the SMART library in case of drift) alongside the launch.html contents.

Of course, the solution also should show the .ready() call and in-app data-retrieval logic (beyond just the .authorize() call). Oh, when I navigate to "Test SMART Launch," I see that's actually where this work gets done. It's probably worth saying that "Enable SMART Launch" is only about the first steps (since I was assuming "Test" would be testing a complete implementation).

Is it possible to inject some JS to show a spoiler solution on-click rather than scrolling, or just linking out to a solution in a new browser tab? (Developers will need to scroll down to see the rest of the tutorial content, so "hiding by scroll" doesn't quite work out here.) Or, I like what you did later; might be worth following the same pattern from step (9):
image

Screenshots: visually, maybe try zooming or cropping given the amount of whitespace
image

Write a CDS Hooks Service

needs to leave the EHR in order to use the app (after remembering how to find it).

In a related PR I'll propose a wording tweak; want to account for the fact that SMART launched apps can still embed in the EHR (so you don't have to leave), but remembering when a given app applies and thinking to launch it... that's cognitive load.

CDS Hooks Testing

This in-depth coverage is great!

Any reason colors alternate like:

image

which should be somewhere on http://localhost

I really do like the hinting and "think-for-yourself" steps here. But I also think we need to show answers somehow. A kind of "reveal answer" button would be great. Incidentally, over the weekend I watched https://youtu.be/g1ib43q3uXQ which gets into some of the benefits of "think-for-yourself" vs "here's-how-to-do", and it was real food for thought. (I grew up very much in the self-guided model, which I never took the time to question.)

jmandel added a commit that referenced this issue Dec 11, 2019
A few tiny tweaks easier to propose directly here...
barabo added a commit that referenced this issue Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants