-
Notifications
You must be signed in to change notification settings - Fork 30
Applies switch compontent #1941
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 |
I'm worried about having badges inside of tabs especially when we implement #1709 and the popovers are triggered by a click instead of a hover (then would clicking the badge both open a popover and switch the tab?). This is the reason we initially proposed just using the content in the badge as plain text instead of actually rendering the badge. @shainaraskas @florent-leborgne keep me honest! (Also if you missed the comment suggesting we use a new directive instead of just adding on to the existing tabs directive: #1436 (comment)) |
I can disable the popover, as well as the current tooltip in tabs. Np. I can also remove the borders.. but IMO it's easier to recognize as applies_to |
fixed in e2cbae5 |
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.
Heavily in favor of a new directive instead of grafting this onto regular tabs.
This allows us to improve the UX in the future and ensure the applies to in this directive are visually distinctive.
I can live with them being not visually identifiable as applies to information in the interim.
I do think when used to distinguish between deployment types it hides an content organization smell and might go against @florent-leborgne simplification initiative. That might be me being ignorant to all the edgecases out there :) By making it a separate control if we need to constrain this to version/availability dimensions only it will be easier to update. |
Thanks @reakaleek! This approach makes sense to me -- it will allow us start standardizing how we structure this kind of information in the source files ahead of the 9.2 release while giving us the flexibility to update what the rendered result looks like when we have dedicated design support. If merging this can wait until tomorrow, I'd like @florent-leborgne to have a chance to comment on the approach and the name of the directives (
Tabs used to distinguish between deployment types are mostly in the content that the admin docs team owns, and I think @shainaraskas has a similar view on whether we should use this in those cases. 🙂 |
docs/syntax/applies-switch.md
Outdated
|
||
## Automatic grouping | ||
|
||
Applies switches are automatically grouped together by default. This means all applies switches on a page will sync with each other - when you select a version in one switch, all other switches will automatically switch to the same version. |
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.
This means all applies switches on a page will sync with each other
Amazing.
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.
Pull Request Overview
This PR introduces a new applies-switch component system that creates tabbed content with applies_to badges instead of text titles. The feature enables content that varies by deployment type, version, or applicability criteria to be displayed in a switch-like interface.
Key changes:
- Implements new applies-switch and applies-item directives for Markdown processing
- Adds corresponding HTML rendering and TypeScript functionality for interactive switching
- Includes comprehensive test coverage and documentation
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs |
Comprehensive test suite for the new applies-switch and applies-item directives |
src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs |
HTML rendering support for applies-switch components with YAML parsing |
src/Elastic.Markdown/Myst/Directives/DirectiveBlockParser.cs |
Parser registration for new applies-switch and applies-item directives |
src/Elastic.Markdown/Myst/Directives/AppliesSwitch/* |
Core implementation classes and views for the applies-switch functionality |
src/Elastic.Markdown/Myst/Components/ApplicableToViewModel.cs |
Added ShowTooltip property for flexible tooltip control |
src/Elastic.Documentation.Site/Assets/* |
CSS styling and TypeScript logic for interactive applies-switch behavior |
docs/syntax/applies-switch.md |
Complete documentation and usage examples for the new feature |
Comments suppressed due to low confidence (1)
@layer components { | ||
.applies-switch { | ||
@apply relative mt-4 flex flex-wrap overflow-hidden; |
Copilot
AI
Sep 24, 2025
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.
This file appears to be a duplicate of applies-switch.css with minor differences. Having two CSS files with similar content creates maintenance overhead. Consider consolidating into one file or clearly documenting the differences.
Copilot uses AI. Check for mistakes.
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.
This works for me. Just to note, we'll likely need to evolve the component itself as we progress with the cumulative docs initiative. If we're all good with this idea, I'm good with it too :) If this raises any concern, we can chat about it @Mpdreamz @reakaleek
Thanks for implementing this, it was much needed as we're starting to have more minor releases out and cases for it.
Part of #1436
Changes
Adds a new
applies-switch
andapplies-item
compontent.Usage
Screenshot