-
Notifications
You must be signed in to change notification settings - Fork 174
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
categories for documentation #907
categories for documentation #907
Conversation
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.
Good work! Just two small comments
docs/navigation-advanced.md
Outdated
|
||
The view groups feature also offers out-of-the-box caching. Each time you navigate to another view group, either a new iframe is created or it is reused if already exists. In both cases, the iframe you are navigating from becomes hidden and is available for you to use again. If you navigate back to the first iframe and it should be updated with new data, such when a new entry was added in the second iframe and you want to display it in a table in the first iframe, you must define a **preloadUrl** parameter for the view group under **navigation.viewGroupSettings**. | ||
|
||
You can also preload view groups. You just need to define which URL you want to preload, and Luigi will preload the view after some user interactions when the browser is most likely to be idle. This option is active by default, but you can deactivate it with a [configuration flag](navigation-parameters-reference.md#node-parameters). |
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 should show name the configuration flag here, else the user has no clue which configuration flag we are talking about. I also am not 100% sure, maybe it is preloadViewUrl (bool) in /navigation-parameters-reference.md#node-navigation-parameters
?
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.
Thank you for the comment! I think the configuration flag for this is preloadViewGroup, so I changed the doc.
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.
Hi Alex. A lot of work has been done here. Good job :)
Please check my comments below.
91ece33
to
12fa0f6
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.
Added comments :)
docs/README.md
Outdated
|
||
## Luigi Client | ||
> **TIP:** The [Luigi Fiddle](https://fiddle.luigi-project.io) allows you to configure a very simple application and get a feel for the process. |
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.
Is it the Luigi Fiddle
or just Luigi Fiddle
- both may be correct, so just asking :)
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.
Normally it's Luigi Fiddle
or the Luigi Fiddle page/website
, so you're right 😄
docs/navigation-advanced.md
Outdated
This document shows you how to configure the following Luigi features: | ||
|
||
* [View groups](#view-groups) | ||
* [Create a dynamically changeable path](#create-a-dynamic-path) |
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 wondering if we can switch it to just Dynamic paths
- to make the points match.
docs/navigation-advanced.md
Outdated
1. Define the **viewGroup** parameter for any navigation node. | ||
2. Children of that node will automatically be considered part of the same view group. |
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 these two do not go well with the introductory line - the first one starting with define
is fine :) but the second one is not a step. I think maybe it's better to have it in one sentence?
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.
Agreed! 🙂
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
Co-Authored-By: Barbara Szwarc <[email protected]>
…nova/luigi into documentation-categories
Description
Changes proposed in this pull request:
Related issue(s)
#766, #906