docs: update grid pro pages + new content - PTL-1517 #1969
Conversation
MrBrunoWolff
requested review from
wjhendry and
jay-taylerson
as code owners
There was a problem hiding this comment.
Can you move the "Getting Started" section to the bottom of the file please, and replace it with a link to it saying that for projects created with Genesis Create and genx that it's already installed, but for legacy projects follow this link for manual installation
There was a problem hiding this comment.
do we have the same for other things like forms/charts/entity management?
If not, I'd keep the current standard for now + maybe we can come with a text to highlight the "pre-setup" on Create/Seed scenarios.
There was a problem hiding this comment.
Yes, though they're simple so they dont' even require the extra step. I will address this in a follow up PR
| import GridProExample from './grid-pro_example.js' | ||
| import Tabs from '@theme/Tabs'; | ||
| import TabItem from '@theme/TabItem'; | ||
|
|
||
| # Grid Pro | ||
|
|
||
| The `@genesislcap/grid-pro` package provides a collection of grid-related components and utilities for Genesis applications. |
There was a problem hiding this comment.
Do we need to mention it wraps ag-grid here considering that we externally link to it multiple times?
There was a problem hiding this comment.
not sure what you mean by the place/code you're tagging this
at index.md I don't really say "we wrap AG" but just link/mention related features
There was a problem hiding this comment.
yes that is my point, we link out to ag grid before really explaining that we're wrapping it, I think currently it implies we're compatiable but we're rolling our own implementation of a grid. Happy to leave this though
docs/001_develop/03_client-capabilities/005_grids/003_grid-pro/grid-pro_02_datasources.mdx
Show resolved
Hide resolved
| export class ConnectedGridServerSideExample extends GenesisElement {} | ||
| ``` | ||
|
|
||
| ## Common Attributes |
There was a problem hiding this comment.
These attribute tables are good but they don't follow the format of every other API section
I know we're short on time now so happy for this to go in if you think you can't do it, and then you or I can tweak it when we get the chance
There was a problem hiding this comment.
I will address in a follow up PR
There was a problem hiding this comment.
I assume you know but just pointing out for clarity - need to elaborate on this file inluding a full API section and examples for React and Angular
There was a problem hiding this comment.
why do we need examples for all on this one? When it's a standard HTML template that should be common across all of those
There was a problem hiding this comment.
- It's not a standard HTML template, it's using property bindings which are Genesis sytnax specific
- It's just to follow the format of every other document. This contains the information but presented differently to everything else, we had agreed a format
I will address this in a subsequent PR
There was a problem hiding this comment.
This is a really good file. If possible it would be good to get live examples. Again we need the examples conerted to React and Angular too
There was a problem hiding this comment.
same here, do we need the whole page written 3 times? I've updated with specific bits that are framework-agnostic.
There was a problem hiding this comment.
No I think you've done a good job now of making this file framework agnostic so no need. Thanks!
…s/docs into bw/ptl-1517
There was a problem hiding this comment.
@matteematt please have another look, this should be good for the beta/dec5 release.
@skawian @SzymonZur I've added all the "renderers" but editable and all the editors are not 100%, if you could have a look too, I believe you worked on this.. so it should be a bit easier to spot issues (the renderers I created are looking good)
Also, had issues with the current useEffect approach.. "live grid-pro" doesn't really work with that but could be a silly mistake too, went with a "load grid" approach, which I like for the renderers page but not too much for the index.
--
The above should be ok to be addressed in a follow-up too, PR is already too big as it is.
There was a problem hiding this comment.
Thanks for the changes, looking good now. I am going to merge this as is so we have the pages ready and then I will start on a follow up PR addressing the comments I left, and I will also try look at seeing if we can get the grid to inialise in the live examples without using the button
Thanks for the effort getting this in
| export class ConnectedGridServerSideExample extends GenesisElement {} | ||
| ``` | ||
|
|
||
| ## Common Attributes |
There was a problem hiding this comment.
I will address in a follow up PR
There was a problem hiding this comment.
Yes, though they're simple so they dont' even require the extra step. I will address this in a follow up PR
| import GridProExample from './grid-pro_example.js' | ||
| import Tabs from '@theme/Tabs'; | ||
| import TabItem from '@theme/TabItem'; | ||
|
|
||
| # Grid Pro | ||
|
|
||
| The `@genesislcap/grid-pro` package provides a collection of grid-related components and utilities for Genesis applications. |
There was a problem hiding this comment.
yes that is my point, we link out to ag grid before really explaining that we're wrapping it, I think currently it implies we're compatiable but we're rolling our own implementation of a grid. Happy to leave this though
There was a problem hiding this comment.
- It's not a standard HTML template, it's using property bindings which are Genesis sytnax specific
- It's just to follow the format of every other document. This contains the information but presented differently to everything else, we had agreed a format
I will address this in a subsequent PR
There was a problem hiding this comment.
No I think you've done a good job now of making this file framework agnostic so no need. Thanks!
Do the changes you have made apply to both Current and Previous versions?
No
Have you done a trial build to check all new or changed links?
Yes
Is there anything else you would like us to know?
No