-
Notifications
You must be signed in to change notification settings - Fork 0
PB-1796: Format and organize pages in Visualize Data section #26
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
38117e8
to
6551415
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.
Monster PR with monster comment section 😅 Let's aim for smaller PRs next time.
I see that the pages are connected but for some I would have appreciated to focus just on one and then take the lessons from there for the next page. Smaller iterations deliver constant value while having it in one big chunk keeps things in limbo for longer.
There are many typos and weird English that we can easily catch using some LLM before one of us reviews a PR.
My biggest concern, however, are the introduction sentences. I know that we don't aim to improve this significantly regarding content. But I feel like at least the first few sentences should be a bit more inviting than "implementation of specs XY".
What I liked though, is that we went back to have a separate XYZ page and that there are runnable examples everywhere.
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.
Mostly ok, some points still open
docs/wmts.md
Outdated
|
||
### Example | ||
<ApiCodeBlock url="https://wmts.geo.admin.ch/1.0.0/WMTSCapabilities.xml?<Lang>" method="GET" /> |
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, fine. If you see the difference I probably did something wrong.
So it's also wrong on the WMS page and we need to adapt? There it says for GetCapabilities &LANG=<Lang>
instead of &lang=<Lang>
.
There are other endpoints that have the query parameters in all caps, e.g. GetFeatureInfo. Is that even correct or are they all lower case? Weird...
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.
It's all good now except for this one comment about the uppercase/lowercase query parameters (LANG
vs lang
etc): #26 (comment).
That's quite an important one. Let's fix that and we are good to merge 🥳
This creates a structure for the Visualize Data pages and organizes the content that was present in these pages. It also updates the Side Menu with a new proposition of organization for these pages.