Combine applies_to badges with the same key #1900
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR combines applies_to badges with the same key into a single badge to reduce redundancy in the UI. The combined badge shows the lifecycle closest to the current version and combines tooltip text from all lifecycles.
- Combines multiple applies_to badges with identical keys into a single badge display
- Implements primary applicability selection logic that prioritizes available versions over future ones
- Modifies tooltip text to include combined information from all lifecycles
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ApplicableToViewModel.cs | Adds badge combination logic and primary applicability selection |
| ApplicabilityRenderer.cs | Implements combined tooltip text generation and primary selection |
| ApplicabilityItem.cs | Updates record structure to support primary applicability |
| ApplicableToComponent.cshtml | Updates template to use primary applicability |
| Test files | Updates test expectations for combined badge behavior |
| ApplicabilityMappings.cs | Minor formatting change |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…ector utility class - Created new ApplicabilitySelector class in Elastic.Documentation.AppliesTo namespace - Removed duplicated GetPrimaryApplicability method from ApplicabilityRenderer and ApplicableToViewModel - Updated both classes to use the new utility class - Improves code reusability and reduces duplication
- Moved ApplicabilitySelector from Elastic.Documentation to Elastic.Markdown.Myst.Components - Added missing using statements for AllVersions and SemVersion - All tests now pass successfully
…o domain layer - Changed parameter from VersioningSystem to SemVersion currentVersion - Moved ApplicabilitySelector back to Elastic.Documentation.AppliesTo namespace - Updated callers to pass versioningSystem.Current - Simplified interface and improved domain separation - All tests pass successfully
|
Currently testing. This is what I'm seeing right now: I think that in order to be shippable standalone and still make sense (rest of the changes will be made separately), we'd need at least the following tweaks made as part of this same PR:
I assume you'd rather keep tooltip information ordering for later but if those are simple to change, I'm also happy to provide input on this here. @reakaleek Thanks for working on this! |
Bold heading and separator
Co-authored-by: Martijn Laarman <Mpdreamz@gmail.com>
Co-authored-by: Martijn Laarman <Mpdreamz@gmail.com>
There was a problem hiding this comment.
Thanks @reakaleek! Just tested and looks good from what I could see :D
Part of #1709
Changes
Follow-ups