console
console copied to clipboard
openshift
CONSOLE-3166, Bug 2116973: Refactor nav components and console app nav extensions
- Replace all console app nav extension
resourceproperties with appropriatemodel+startsWithproperties - Add a deprecated message to all static nav extension types and remove references to legacy nav components
- Update
connectToModel,connectToPlural, andFirehoseso that they resolve k8s models for core APIgroup~version~kindreferences - Refactor
NavLink*class components toNavItem*function components to remove inheritance pattern - Remove
NavLinkRootcomponent and include logic in refactoredNavItem*components - Refactor
PluginNavItemstoPluginNavItemand map extensions from the parent - General naming improvements
- Improve nav-related util function naming and logic
I'm kinda shocked our extensions were so wrong, thanks for cleaning those up 👍
Yeah, that was leftover tech debt from one of my previous stories. Things got ugly 😆
I was doing some edge case testing and I'm not sure if I am just blundering my configuration or not -- but I think startsWith isn't doing what it's supposed to be doing 🤔
diff --git a/dynamic-demo-plugin/console-extensions.json b/dynamic-demo-plugin/console-extensions.json
index 57ce393b66..3bc3756d16 100644
--- a/dynamic-demo-plugin/console-extensions.json
+++ b/dynamic-demo-plugin/console-extensions.json
@@ -89,6 +89,7 @@
"perspective": "admin",
"section": "admin-demo-section",
"name": "%plugin__console-demo-plugin~Dynamic Nav 1%",
+ "startsWith": ["dynamic-route"],
"href": "/dynamic-route-1"
}
},
This doesn't work... I'd expect both dynamic-route hrefs to highlight when I click on and navigate to /dynamic-route-2 -- if I make it "startsWith": ["dynamic-route-2"], it works, but that seems too specific.
I was doing some edge case testing and I'm not sure if I am just blundering my configuration or not -- but I think
startsWithisn't doing what it's supposed to be doing 🤔diff --git a/dynamic-demo-plugin/console-extensions.json b/dynamic-demo-plugin/console-extensions.json index 57ce393b66..3bc3756d16 100644 --- a/dynamic-demo-plugin/console-extensions.json +++ b/dynamic-demo-plugin/console-extensions.json @@ -89,6 +89,7 @@ "perspective": "admin", "section": "admin-demo-section", "name": "%plugin__console-demo-plugin~Dynamic Nav 1%", + "startsWith": ["dynamic-route"], "href": "/dynamic-route-1" } },This doesn't work... I'd expect both dynamic-route hrefs to highlight when I click on and navigate to
/dynamic-route-2-- if I make it"startsWith": ["dynamic-route-2"],it works, but that seems too specific.
The API docs state the following on this property:
Mark this item as active when the URL starts with one of these paths.
I interpreted this to mean that it would match on entire path segments and not partial strings.
The API docs state the following on this property:
Mark this item as active when the URL starts with one of these paths.
I interpreted this to mean that it would match on entire path segments and not partial strings.
That being said -- if I recall correctly, it worked before your PR and now does not. Bug caused or bug fixed? Up to debate I guess. I find it less likely to be a bug fixed as its a loss of functionality for no specific reason -- it's ambiguous at best I feel.
If you want to claim this is the way it should work, I'd make the definition behind the property more specific as to remove the ambiguity. string.startsWith() (MDN ref) doesn't assume anything other than it starts with the string. So baring more specific information, I'd expect similar functionality as a developer.
As long as it is documented accurately, we can claim not-a-bug if someone has an issue with it and mull over the enhancement as needed.
@andrewballantyne I just pushed up some changes that address your comments.
QE Approver: /assign @yapei
Docs Approver: /assign @opayne1
PX Approver: /assign @RickJWagner
@TheRealJon: This pull request references Bugzilla bug 2116973, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.
3 validation(s) were run on this bug
- bug is open, matching expected state (open)
- bug target release (4.12.0) matches configured target release for branch (4.12.0)
- bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Requesting review from QA contact: /cc @yapei
In response to this:
CONSOLE-3166, Bug 2116973: Refactor nav components and console app nav extensions
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
/retest-required
Remaining retests: 2 against base HEAD 1ba5f5a093cf2a33888b6f6cd0c287d6d26d7dd3 and 8 for PR HEAD eb5e1e8f93602f08080f0ffd2a3ed8c4c526d984 in total
/retest-required
Remaining retests: 1 against base HEAD 1ba5f5a093cf2a33888b6f6cd0c287d6d26d7dd3 and 7 for PR HEAD eb5e1e8f93602f08080f0ffd2a3ed8c4c526d984 in total