-
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
[Multiple Datasource] Allow top nav menu to mount data source menu for use case when both menus are mounted #6268
Conversation
Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6268 +/- ##
==========================================
+ Coverage 67.42% 67.44% +0.02%
==========================================
Files 3362 3362
Lines 65258 65261 +3
Branches 10524 10526 +2
==========================================
+ Hits 43999 44017 +18
+ Misses 18701 18688 -13
+ Partials 2558 2556 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -97,10 +110,12 @@ export function TopNavMenu(props: TopNavMenuProps): ReactElement | null { | |||
} | |||
|
|||
function renderMenu(className: string): ReactElement | null { | |||
if (!config || config.length === 0) return null; | |||
if ((!config || config.length === 0) && (!showDataSourceMenu || !dataSourceMenuConfig)) |
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.
There is typo in this file with line#124 // Validate presense of all required fields
, presense
should be presence
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.
Yeah, fixed the comment
Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
…r use case when both menus are mounted (#6268) * allow top nav menu to mount data source menu Signed-off-by: Lu Yu <nluyu@amazon.com> * add change log Signed-off-by: Lu Yu <nluyu@amazon.com> * update snapshots Signed-off-by: Lu Yu <nluyu@amazon.com> * update snapshot Signed-off-by: Lu Yu <nluyu@amazon.com> --------- Signed-off-by: Lu Yu <nluyu@amazon.com> (cherry picked from commit e84582c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
…r use case when both menus are mounted (#6268) (#6273) * allow top nav menu to mount data source menu Signed-off-by: Lu Yu <nluyu@amazon.com> * add change log Signed-off-by: Lu Yu <nluyu@amazon.com> * update snapshots Signed-off-by: Lu Yu <nluyu@amazon.com> * update snapshot Signed-off-by: Lu Yu <nluyu@amazon.com> --------- Signed-off-by: Lu Yu <nluyu@amazon.com> (cherry picked from commit e84582c) 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>
Description
This change allows top nav menu to mount data source menu for use case when both menus are mounted.
Issues Resolved
Fixes #6229
Screenshot
topnavmenu.mp4
Testing the changes
The following steps were performed in the recording above:
Check List
yarn test:jest
yarn test:jest_integration