esphome-docs icon indicating copy to clipboard operation
esphome-docs copied to clipboard

Initial LVGL doc for ESPHome

Open nagyrobi opened this issue 11 months ago • 13 comments

Description:

Related issue (if applicable): N/A

Pull request in esphome with YAML changes (if applicable): esphome/esphome#6363

Doc preview at: https://deploy-preview-3678--esphome.netlify.app/

Checklist:

  • [x] I am merging into next because this is new documentation that has a matching pull-request in esphome as linked above.
    or

  • [ ] I am merging into current because this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.

  • [x] Link added in /index.rst when creating new documents for new components or cookbook.

nagyrobi avatar Mar 12 '24 09:03 nagyrobi

Deploy Preview for esphome ready!

Name Link
Latest commit 3d5bb6a0dc34f01a13c416831a7295e02db2540f
Latest deploy log https://app.netlify.com/sites/esphome/deploys/66b93a39366a0300082275e2
Deploy Preview https://deploy-preview-3678--esphome.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Mar 12 '24 09:03 netlify[bot]

This is an amazing document & effort! 🍻 As you requested, I had a look and left...a few...comments. 😅 This is as far as I was able to get today; I'll continue tomorrow. 😇

Thanks!

Content is still being added, as since Jesse didn't want to merge it in the most recent beta Clyde decided to add a few more useful things.

nagyrobi avatar May 09 '24 13:05 nagyrobi

@nagyrobi I think you may have missed some of my comments since GH hides them to "shrink" the page. 😅

kbx81 avatar May 09 '24 22:05 kbx81

@nagyrobi I think you may have missed some of my comments since GH hides them to "shrink" the page. 😅

I keep running into that

nagyrobi avatar May 10 '24 06:05 nagyrobi

Ok I need another break 😅

Told ya this is massive :) @kbx81 Thank you for your observations, I accepted most of them. Two things:

  • I'd really like to keep the "Specific" term at each widget configuration opctions/actions/triggers to emphasise that these apply only to that widget and not the others. By the end of the document there are "unviersal" actions/triggers which appear also in the TOC.
  • I don't want to add these as sub-headings as they will overwhelm the TOC on the left. It should be an exception for this page, given the size of it. It's enough if the widgets themselves are accessible from the TOC, the rest will be within the screen. They are the same level as the Examples.

nagyrobi avatar May 10 '24 07:05 nagyrobi

Ok I need another break 😅

Told ya this is massive :) @kbx81 Thank you for your observations, I accepted most of them. Two things:

  • I'd really like to keep the "Specific" term at each widget configuration opctions/actions/triggers to emphasise that these apply only to that widget and not the others. By the end of the document there are "unviersal" actions/triggers which appear also in the TOC.

Ah, I hadn't gotten that far so I didn't realize that. Still, it feels...extra wordy. I think I understand what you're getting at, but, as each widget type has its own heading/section, the word "specific" isn't adding any value. Explained another way: if I scroll to a random point in the document, and I don't realize what part I'm looking at, having the word "specific" there isn't going to help me. It's just more words on the page. In addition, you've got a "Common properties" section at the top of the "Widgets" section, which should imply that the subsequent sections describe properties that are unique to that specific widget type.

  • I don't want to add these as sub-headings as they will overwhelm the TOC on the left. It should be an exception for this page, given the size of it. It's enough if the widgets themselves are accessible from the TOC, the rest will be within the screen. They are the same level as the Examples.

That thought had crossed my mind. 😄 I think the main LVGL document needs to be broken into multiple pages -- not a lot, maybe just 2-3. It's really, really long, and the browser/GH is lagging due to the size as I'm reviewing. Just a thought. 😇

kbx81 avatar May 10 '24 09:05 kbx81

Keep in mind that, thus far, I'm approaching this from these perspectives:

  • I've never used LVGL before, but I'm quite familiar with embedded hardware, software and UI concepts/design.
  • I'm reading this to learn about LVGL so I can use it in a future project with ESPHome.
  • I'm looking for simple, coherent explanations with as few words as possible while maintaining grammatical correctness.
  • I want to eliminate poorly-worded, incomplete or confusing explanations that take extra time to parse or require excessive thinking or researching to understand them.
  • I expect the LVGL documentation to be as consistent as reasonably possible with the rest of the documentation on the ESPHome site, which I am already quite familiar with.

kbx81 avatar May 10 '24 09:05 kbx81

browser/GH is lagging

Only the netlify previews are lagging, they are not served out by the same scale CDN as the main site. Any other PR's preview lags the same compared to the live site.

nagyrobi avatar May 10 '24 10:05 nagyrobi

having the word "specific" there isn't going to help me

It is going to make you realize, that you only see a small part of what's possible. These days people read less and less. They always jump to the examples and just do copy-paste. Instead of reading the doc, they watch third party videos which only show certain things out of the whole.

Having such keywords put in the text have the effect of at least making wonder the first time passer by, that there's still information worth looking after.

But I removed them, anyway.

nagyrobi avatar May 10 '24 11:05 nagyrobi

I still think you should break the main lvgl document into more sections.

* The _core_ lvgl explaining the main component and the basic info of the widgets.

* And the _widgets_
  The main `index.rst` file should show all the widgets separately so we have way to directly jump from the main index page.

No, that would overwhelm the main index a lot. Also, the widgets themselves are not separate components. lvgl is a component, the widgets are just options (aka "variables") of it. Parts which represent separate components (the lvgl switch, sensor etc) already got separate pages, just like any other ESPHome platform which breaks into separate components. And those appear in the main index too. So how it looks now is conforming, each component (plus the cookbook) has its own page, but within the page navigation is easy and straightforward.

nagyrobi avatar May 10 '24 15:05 nagyrobi

A few corrections and rephrasing, but this is great! Very useful examples. 😄

Will add this week two more: one to demonstrate the layout functionality, and one to show a weather panel updating from HA.

nagyrobi avatar May 13 '24 09:05 nagyrobi

Just a few minor touch-ups 😄

Great, thanks, keep 'em coming!

nagyrobi avatar May 15 '24 09:05 nagyrobi

[!IMPORTANT]

Review Skipped

Auto reviews are disabled on base/target branches other than the default branch. Please add the base/target branch pattern to the list of additional branches to be reviewed in the settings.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to full the review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar May 28 '24 07:05 coderabbitai[bot]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

esphome[bot] avatar Aug 05 '24 09:08 esphome[bot]

@nagyrobi I was discussing this a bit with Jesse and we have a couple thoughts/requests:

  • Let's break all of the cookbook stuff into its own PR -- this will help reduce the size of this PR.
  • Given the sheer size of the main document, we think it would make sense to break it into multiple documents, similar to what I've done with the Automations page. Perhaps a structure like this:
    • /components/lvgl/index.rst - LVGL Introduction/Overview (and Basics?), with links to all other LVGL docs
    • /components/lvgl/main_configuration.rst - LVGL Main Configuration (and Basics, if not in above doc)
    • /components/lvgl/widgets.rst - LVGL Widgets
    • /components/lvgl/automations.rst - LVGL Actions/Conditions/Triggers

Each document can of course link back to the other documents to make navigation easier.

kbx81 avatar Aug 05 '24 09:08 kbx81

@kbx81 @jesserockz I find it harder to use in this specific case because of many nested stuff. Eg. almost every widget has specific Actions/Conditions/Triggers in addition to generic ones. One clicks to compare the generic ones vs the specific ones, has to change page and navigate back-forth with the browser between two pages, whilst if they were on the same page it's just scrolling up and down or Ctrl+F...

Similarly many Styling options ("variables") are crosslinked back from the widgets to the common section, which could eventually end on a different page, which would again, make it harder to use.

What's wrong with the fact that the document itself is long? It's still easier to be used all-in-one instead of having to constantly jump between the pages... A separate page with the wigdets makes no sense without having the other stuff prepared around them, and they don't apply anywhere else...

Now everything is nicely structured and all accessible from within a single TOC with a single click...

Having the components themselves as separate pages makes sense and it's already like that...

Agree with the cookbook, moving that to separate PR.

nagyrobi avatar Aug 05 '24 10:08 nagyrobi

PR with the cookbook: https://github.com/esphome/esphome-docs/pull/4110

nagyrobi avatar Aug 05 '24 13:08 nagyrobi

Expanding on what Jesse mentioned....

@kbx81 @jesserockz I find it harder to use in this specific case because of many nested stuff. Eg. almost every widget has specific Actions/Conditions/Triggers in addition to generic ones. One clicks to compare the generic ones vs the specific ones, has to change page and navigate back-forth with the browser between two pages, whilst if they were on the same page it's just scrolling up and down or Ctrl+F...

A couple of thoughts:

  1. I don't know anybody today who is only using a single browser window with a single tab for documentation/research. I would argue that having multiple windows/tabs open should be expected and not something we assume people will want to avoid.
  2. When the page is so long, scrolling up and down is quite tedious. In this case, yes, there is a link from (for example) any given widget to the Style properties section but there is not a link from the Style properties section back to the widget. Yes, the TOC on the left potentially solves for this but it may not always be visible -- consider mobile devices.

Ctrl+F works but tends to be clunky because I may get dozens of hits which makes this approach a lot less useful. Ctrl+F also may not be available/usable on mobile devices such as phones or tablets.

Similarly many Styling options ("variables") are crosslinked back from the widgets to the common section, which could eventually end on a different page, which would again, make it harder to use.

Again, everybody is using browser tabs today -- and lots of them, at that.

What's wrong with the fact that the document itself is long?

Have you looked a this on a mobile device where the TOC on the left is not visible? It's quite difficult to navigate such a huge document on such a device. I realize most folks will be viewing this on a desktop-class system, but we should still try to organize/structure it in a way so it's still reasonably friendly on mobile devices, too. I know for a fact that some users view our documentation this way.

In general, the problem with such a big page is scrolling around on it -- if I don't open multiple tabs/windows, I'm going to constantly be losing my place, which is distracting. The TOC on the left and Ctrl+F may help with this, but we shouldn't assume those tools are always visible/available.

It's still easier to be used all-in-one instead of having to constantly jump between the pages... A separate page with the wigdets makes no sense without having the other stuff prepared around them, and they don't apply anywhere else... Having the components themselves as separate pages makes sense and it's already like that...

We can still organize it in a way that makes sense. See Jesse's comment/suggestion above. Sprinkling in links across the pages/sections helps a lot. I use Ctrl+Click to open links in a new tab all the time. When there aren't links, my life is actually more difficult for this reason. Put differently, instead of scrolling all over a given document/page, I would rather have links that I can Ctrl+Click to navigate across sections. It's just easier.

Agree with the cookbook, moving that to separate PR.

Thank you! 🍻

kbx81 avatar Aug 05 '24 23:08 kbx81

I marked unresolved some of my earlier comments/feedback because the suggested changes weren't made and no other comment/feedback was left.

kbx81 avatar Aug 05 '24 23:08 kbx81

Due to too many conflicts, the doc has been resumbmitted as a new PR: https://github.com/esphome/esphome-docs/pull/4115

nagyrobi avatar Aug 06 '24 11:08 nagyrobi

Please don't just throw away this PR because of some merge conflicts that took me all of 30 seconds to fix.

jesserockz avatar Aug 06 '24 21:08 jesserockz