-
Notifications
You must be signed in to change notification settings - Fork 41
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
DOCS-2481: Add motor how to #3036
DOCS-2481: Add motor how to #3036
Conversation
Overall readability score: 53.04 (🟢 +0.03)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
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.
I like the format with the expanders.
I don't think this currently provides enough detail for people to accomplish this task if they've just created an account and haven't done anything else yet. I'm also a bit sceptical whether we can actually do this in 60 seconds, how do you feel about that? Do you think that premise makes sense?
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.
I'm not sure I understand the change here - Does this change anythign aside from the cursor pointer right? We can discuss changing this but then we should change the expand shortcode so that it gets changed everywhere for consistency.
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 uses <details>
and <summary>
HTML elements instead of <div>
. The browser automatically expands collapsed content when users search using Command + F.
This way users can search through content even when they have not expanded a section manually. In the original expand.html
, collapsed content is not searchable by default, so users had to manually expand sections to search within them.
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.
!!!!!!!!!!!!!
I didn't know that - that is so cool!! I wonder if we can even use that to make tabs searchable. Awesome!
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.
Ohh I will definitely be trying this out today..!
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.
makes me wonder whether we can also make them open up as we scroll through the page. But we can look into that separately
docs/use-cases/control-motor.md
Outdated
|
||
First, [create a machine](/cloud/machines/#add-a-new-machine) if you haven't yet. | ||
|
||
Then, [add a board component](/components/board/), such as a [Raspberry Pi board](/components/board/pi/). |
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.
hmmm do you think you could actually get this done in 60 seconds. We may need to adjust this. We may also need to add a requirements section that says you need to have a board with a supported OS installed and a motor connected.
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.
I think 2-3 minutes is more realistic, especially if adding a new machine is a step. I also like the idea of adding a requirements section. I can adjust this to 2 minutes and test it out a bit on an actual motor. What do you think?
docs/use-cases/control-motor.md
Outdated
|
||
You can use [the Viam mobile app](/fleet/#the-viam-mobile-app) to control your motor's speed and direction directly from your smart device. | ||
|
||
{{<gif webm_src="/fleet/mobile-app-control.webm" mp4_src="/fleet/mobile-app-control.mp4" alt="Using the control interface under the locations tab on the Viam mobile app" max-width="300px">}} |
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.
I'm not sure whether this will be confusing given people only have a motor configured :/ . Maybe we crop this?
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.
I recorded a new gif, but I could also just crop this one!
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.
We want to add a new quickstarts section in the get-started section here: https://docs-test.viam.dev/3036/get-started/, so this should be under tutorials
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 should've said this should not be under tutorials - sorry. I'll move this and then merge
layouts/shortcodes/expand.html
Outdated
</summary> | ||
<div class="expand-content"> | ||
{{- if (findRE "(gif|mp4)_src" .Inner) -}} | ||
<div class="media-wrapper"> |
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.
what does this media-wrapper do here?
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.
Thanks for catching this, it was a leftover class from attempting to figure out an issue with the 'alignright'" class.
There was a rendering issue with the tabs shortcode and the gif overlapping, but using the 'clear' attribute worked. This will be gone in the next commit!
layouts/shortcodes/expand.html
Outdated
</span> | ||
</summary> | ||
<div class="expand-content"> | ||
{{- if (findRE "(gif|mp4)_src" .Inner) -}} |
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.
and what does this do? It looks for gifs and mp4s and if so wraps it with safeHTML and otherwise uses markdownify? Is that needed for edgecases?
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.
Yes! The purpose of this conditional was to ensure that the content of the HTML element 'details' is processed as markdown when it's not a gif or mp4.
Prior to adding the markdownify conditional, the hyperlinks would not render as hyperlinks within the new shortcode. Since the original shortcode uses 'div' in the original shortcode, this didn't need to be explicitly written as it seems that 'div' takes care of this for us.
|
||
# motor-1 | ||
motor_1 = Motor.from_robot(machine, "motor-1") | ||
motor_1_return_value = await motor_1.is_moving() |
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.
can you use a different method here? This will not move the motor but just return that it's not moving.
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.
Same for go
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.
Ok this looks good - a few small changes, then I think this is ready to test on someone :)
8c42988
to
ed0712f
Compare
5091591
to
b73a2e2
Compare
b73a2e2
to
abd4c02
Compare
You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/3036 |
This PR introduces a stylistically different approach from our other how-to pages. I am happy to make any changes as needed; this shortcode is a stylistic option I'm exploring.
expand-and-search.html
This new shortcode is similar to
expand.html
, but utilizes the<details>
element rather than<div>
. This allows users to use Cmd + F to search through the contents of the expandable steps even when they are collapsed. When the content is searched in this way, the relevant expandable section will automatically open.Purpose
The reasons I am trying the implementation of this style is:
Focused Search
I think this approach benefits users who know exactly what they are looking for. For example, if a user already has configured a board and is looking for a specific function, such as simply going straight to controlling the motor, they can find it easily without sifting through steps they have already done.
Two high-level overviews
This approach also provides a high-level overview of the already quite high-level steps required to achieve functionality. I thought this could be helpful for understanding the overall process before diving into each step in detail.
Simplicity
This approach provides a page that users might not feel overwhelmed or discouraged by due to its simplicity, but that is very subjective.