-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Javascript Get Started with code walkthrough #3234
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
base: master
Are you sure you want to change the base?
Conversation
74523e2
to
a8b53af
Compare
a8b53af
to
63fc110
Compare
f84f5bb
to
15c6114
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good, but I do see some room to polish it up.
- The CSS could be better. We should save this for a follow-up task, but just making note of what I see:
- The font color of the code sample's non-focused areas feels a bit too low-contrast to me, especially since the entire code sample is "greyed out" much of the time. Some of the UI elements like the Environment selector are also low visibility in light mode.
- The columns feel cramped to me and a little uneven too. Especially on my 1440p desktop monitor, there's wasted space due to the max width (1504px) of
.code-walkthrough
elements... I don't know why that's there, but removing it makes the fit a lot better on big monitors. - The margins between elements and the space for line numbers in the code sample feel a bit too small.
- We should add a README to the
js/
folder that sends people to the page where they can download the clean example. We might also want to provide a separate, clean example file in the repository, without the@chunk
comments. That would involve maintenance to keep them from diverging, though, so maybe not. - It seems wrong to have this buried under "Build Applications..." That's something we can and should address later when we move the tutorials around as planned. After we do that, we should also update the
_code-samples/get-started/README.md
file to have the new links to the tutorials.
Otherwise, I noted a few places where the old format or language needs to be updated just a little. This should be good to go with those changes.
Co-authored-by: Rome Reginelli <rome@ripple.com>
Holding off on merging this, as per @amarantha-k's suggestion to wait for feedback from the design team. |
Preview link here