-
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]Refactor authentication type selection in create and edit data source forms #3693
[Multiple DataSource]Refactor authentication type selection in create and edit data source forms #3693
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #3693 +/- ##
==========================================
- Coverage 66.43% 66.38% -0.05%
==========================================
Files 3210 3210
Lines 61677 61676 -1
Branches 9522 9522
==========================================
- Hits 40977 40946 -31
- Misses 18419 18445 +26
- Partials 2281 2285 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Could we just confirm with UX the layout is what's been advised? |
@KrooshalUX Kroosh, could you take a look? |
Assuming this is using |
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.
Just a few comments about the implementation and types. Functionality looks good.
...management/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx
Outdated
Show resolved
Hide resolved
...management/public/components/edit_data_source/components/edit_form/edit_data_source_form.tsx
Outdated
Show resolved
Hide resolved
...ement/public/components/edit_data_source/components/edit_form/edit_data_source_form.test.tsx
Outdated
Show resolved
Hide resolved
...blic/components/create_data_source_wizard/components/create_form/create_data_source_form.tsx
Outdated
Show resolved
Hide resolved
… forms to use EuiSelect instead of EuiRadioGroup for better user experience Signed-off-by: Su <[email protected]>
Signed-off-by: Su <[email protected]>
953bc10
to
a45cfe1
Compare
Signed-off-by: Su <[email protected]>
a45cfe1
to
26fae2a
Compare
@ashwin-pc @kristenTian addressed all comments, and resolve conflict in changelog, plz take another look, thx |
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.
LGTM!
idSelected={this.state.auth.type} | ||
onChange={(id) => this.onChangeAuthType(id)} | ||
value={this.state.auth.type} | ||
onChange={(e) => this.onChangeAuthType(e)} |
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.
onChange={(e) => this.onChangeAuthType(e)} | |
onChange={this.onChangeAuthType} |
nit: not blocking the PR for this though
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.
made the change
CHANGELOG.md
Outdated
@@ -167,6 +167,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) | |||
- [Vis Builder] Removed Hard Coded Strings and Used i18n to transalte([#2867](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2867)) | |||
- [Console] Replace jQuery.ajax with core.http when calling OSD APIs in console ([#3080](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3080)) | |||
- [I18n] Fix Listr type errors and error handlers ([#3629](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3629)) | |||
- [Multiple DataSource]Refactor authentication type selection in create and edit data source forms to use EuiSelect ([#3693](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3693)) |
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.
- [Multiple DataSource]Refactor authentication type selection in create and edit data source forms to use EuiSelect ([#3693](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3693)) | |
- [Multiple DataSource] Present the authentication type choices in a drop-down ([#3693](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3693)) |
How does this sound?
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 sound nicer!
{ value: AuthType.NoAuth, text: 'No authentication' }, | ||
{ value: AuthType.UsernamePasswordType, text: 'Username & Password' }, | ||
{ value: AuthType.SigV4, text: 'AWS SigV4' }, |
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.
Can we use i18n for these?
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.
of course
Signed-off-by: Su <[email protected]>
… and edit data source forms (#3693) * Refactor authentication type selection in create and edit data source forms to use EuiSelect instead of EuiRadioGroup for better user experience Signed-off-by: Su <[email protected]> (cherry picked from commit 44e2e34) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
… and edit data source forms (#3693) (#3790) * Refactor authentication type selection in create and edit data source forms to use EuiSelect instead of EuiRadioGroup for better user experience Signed-off-by: Su <[email protected]> (cherry picked from commit 44e2e34) 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>
… and edit data source forms (opensearch-project#3693) * Refactor authentication type selection in create and edit data source forms to use EuiSelect instead of EuiRadioGroup for better user experience Signed-off-by: Su <[email protected]> Signed-off-by: David Sinclair <[email protected]>
Description
[Multiple DataSource]Refactor authentication type selection in create and edit data source forms to use EuiSelect component
Based on #2110 (comment), I made the UI changes to multiple datasource creation page
Current UI:
Updated UI:
Issues Resolved
#3551
#2110 (comment)
Check List
yarn test:jest
yarn test:jest_integration
yarn test:ftr