-
Notifications
You must be signed in to change notification settings - Fork 916
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
feat: Add rightNavigationButton
component in chrome service for applications to register and add dev tool to top right navigation
#6553
feat: Add rightNavigationButton
component in chrome service for applications to register and add dev tool to top right navigation
#6553
Conversation
❌ Invalid Prefix For Manual Changeset CreationInvalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files. If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description. |
❌ Invalid Prefix For Manual Changeset CreationInvalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files. If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description. |
❌ Invalid Prefix For Manual Changeset CreationInvalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files. If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description. |
registerRightNavigation
in chrome service to reigster standard top right navigation and add dev tool to top right navigation
registerRightNavigation
in chrome service to reigster standard top right navigation and add dev tool to top right navigationregisterRightNavigation
in chrome service to register standard top right navigation and add dev tool to top right navigation
How is this different from |
Signed-off-by: tygao <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
…ons to register and add dev tool to top right navigation. Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
1c0da2a
to
64051a6
Compare
application: core.application, | ||
http: core.http, |
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.
Nit: perhaps CoreStart
will be better for backward-capability.
src/core/public/chrome/constants.ts
Outdated
export enum RightNavigationOrder { | ||
// order of dev tool should be after advance settings | ||
Settings = 1, | ||
DevTool = 2, |
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.
nit: the value could be 10, 20 and leave some gaps between them so that we can add more between settings and devtool without change the value.
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.
That makes sense. Updated.
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.
Approved with a minor comment
const navigateToApp = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) => { | ||
/* Use href and onClick to support "open in new tab" and SPA navigation in the same link */ | ||
if ( | ||
event.button === 0 && // ignore everything but left clicks |
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.
Nit: consider to extract to a function for better readability
const isLeftClickEvent = (event) => event.button === 0;
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.
Updated.
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.
Seems your guys approvals were dismissed because of commits to address comments. Could you guys help add 2.x label and approve again?
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
…lications to register and add dev tool to top right navigation (#6553) * feat: add dev tool to top right navigation Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * test: update component props snapshot Signed-off-by: tygao <[email protected]> * update nav_controls_service.ts Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]> * update dependency props and constants Signed-off-by: tygao <[email protected]> * update nav control test Signed-off-by: tygao <[email protected]> * Update CHANGELOG.md Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]> * test: update snapshots Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * update click handle Signed-off-by: tygao <[email protected]> * update click handle Signed-off-by: tygao <[email protected]> * doc: add doc for registerRightNavigation Signed-off-by: tygao <[email protected]> * Add `rightNavigationButton` component in chrome service for applications to register and add dev tool to top right navigation. Signed-off-by: tygao <[email protected]> * test: update core start type Signed-off-by: tygao <[email protected]> * update RightNavigationOrder value Signed-off-by: tygao <[email protected]> * extract left click function Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]> Co-authored-by: Yulong Ruan <[email protected]> (cherry picked from commit cfe7cd1) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
…lications to register and add dev tool to top right navigation (#6553) (#6684) * feat: add dev tool to top right navigation Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * test: update component props snapshot Signed-off-by: tygao <[email protected]> * update nav_controls_service.ts Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]> * update dependency props and constants Signed-off-by: tygao <[email protected]> * update nav control test Signed-off-by: tygao <[email protected]> * Update CHANGELOG.md Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]> * test: update snapshots Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * update click handle Signed-off-by: tygao <[email protected]> * update click handle Signed-off-by: tygao <[email protected]> * doc: add doc for registerRightNavigation Signed-off-by: tygao <[email protected]> * Add `rightNavigationButton` component in chrome service for applications to register and add dev tool to top right navigation. Signed-off-by: tygao <[email protected]> * test: update core start type Signed-off-by: tygao <[email protected]> * update RightNavigationOrder value Signed-off-by: tygao <[email protected]> * extract left click function Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]> Co-authored-by: Yulong Ruan <[email protected]> (cherry picked from commit cfe7cd1) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…lications to register and add dev tool to top right navigation (opensearch-project#6553) * feat: add dev tool to top right navigation Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * test: update component props snapshot Signed-off-by: tygao <[email protected]> * update nav_controls_service.ts Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]> * update dependency props and constants Signed-off-by: tygao <[email protected]> * update nav control test Signed-off-by: tygao <[email protected]> * Update CHANGELOG.md Co-authored-by: SuZhou-Joe <[email protected]> Signed-off-by: tygao <[email protected]> * test: update snapshots Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * update click handle Signed-off-by: tygao <[email protected]> * update click handle Signed-off-by: tygao <[email protected]> * doc: add doc for registerRightNavigation Signed-off-by: tygao <[email protected]> * Add `rightNavigationButton` component in chrome service for applications to register and add dev tool to top right navigation. Signed-off-by: tygao <[email protected]> * test: update core start type Signed-off-by: tygao <[email protected]> * update RightNavigationOrder value Signed-off-by: tygao <[email protected]> * extract left click function Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> Co-authored-by: SuZhou-Joe <[email protected]> Co-authored-by: Yulong Ruan <[email protected]>
Description
Issues Resolved
#6330
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration