-
Notifications
You must be signed in to change notification settings - Fork 164
Add warning in searchapps page #3566
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
Conversation
🔍 Preview links for changed docs |
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 this type of warning something we typically call out in our docs? I don't remember us adding warning for previous nav changes, including when we removed app search from the nav. I'll defer to @leemthompo if this is necessary or not.
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, just some minor comments
```{applies_to} | ||
serverless: unavailable | ||
``` | ||
::::{warning} |
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 can add applies_to's to admonitions now, so we don't have to spell out the deployment stuff in prose.
See https://elastic.github.io/docs-builder/syntax/admonitions/#applies-to-information
Co-authored-by: Liam Thompson <[email protected]>
I think it makes sense because folks running pre 9.2 deployments will still have access to the feature. It differs from the App Search case, because App Search disappeared in a major bump that coincided with docs migration for 9.0. I'd say it's a 70/30 call, but not a huge difference either way :-) The alternative would be to use applies_to's to say that the feature is available in 9.0-9.1 but unavailable in 9.2. Same message, but less visible and I think the callout edges it in terms of clarity. |
Co-authored-by: Liam Thompson <[email protected]>
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 @ketkee-aryamane
@kderusso |
That's a good idea, much better than just an "We recommended building with search templates instead." In fact we could also have this recommendation at the top of each page TBH. |
I'm a little on the fence for the redirect, because if people are using search applications for the endpoint then that won't be an applicable replacement. |
Right maybe if we are clear that this would be a recommendation for brand new users who haven't yet used any Search |
@ketkee-aryamane I vote we merge this now as is, and we can revisit adding additional recommendations when the API conversation matures |
Linked to #3564
Adds a warning to the UI navigation instructions for Search apps.