Skip to content
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.

Sign up for GitHub

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

[Workspace] Refactor data source association panel #8383

Merged
merged 9 commits into from
Sep 30, 2024
Merged

[Workspace] Refactor data source association panel #8383

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0b2bb1b
refactor association panel
Kapian1234 Sep 30, 2024
0c5b90f
Changeset file for PR #8383 created/updated
opensearch-changeset-bot[bot] Sep 30, 2024
2165f38
resolve some issues
Kapian1234 Sep 30, 2024
bb2363e
merge from origin
Kapian1234 Sep 30, 2024
3aeff67
update title style
Kapian1234 Sep 30, 2024
57bcdff
update title style
Kapian1234 Sep 30, 2024
892fe16
update placeholder
Kapian1234 Sep 30, 2024
416d935
update placeholder
Kapian1234 Sep 30, 2024
b768623
/
Kapian1234 Sep 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/8383.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Refactor data source association panel ([#8383](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8383))
56 changes: 33 additions & 23 deletions src/plugins/workspace/public/components/workspace_creator/workspace_creator_form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@
*/

import React, { useCallback, useRef } from 'react';
import { EuiSpacer, EuiTitle, EuiForm, EuiText, EuiFlexItem, EuiFlexGroup } from '@elastic/eui';
import {
EuiSpacer,
EuiTitle,
EuiForm,
EuiText,
EuiFlexItem,
EuiFlexGroup,
EuiPanel,
} from '@elastic/eui';
import { i18n } from '@osd/i18n';
import {
useWorkspaceForm,
Expand Down Expand Up @@ -110,29 +118,31 @@ export const WorkspaceCreatorForm = (props: WorkspaceCreatorFormProps) => {
{/* SelectDataSourcePanel is only visible for dashboard admin and when data source is enabled*/}
{isDashboardAdmin && isDataSourceEnabled && (
<>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep this fragment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

<EuiTitle
{...generateRightSidebarScrollProps(RightSidebarScrollField.DataSource)}
size="s"
>
<h3>
{i18n.translate('workspace.creator.form.associateDataSourceTitle', {
defaultMessage: 'Associate data sources',
<EuiPanel>
<EuiTitle
Copy link
Contributor

@wanglam wanglam Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use EuiText + h2 as panel title like this to align with the guidance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

{...generateRightSidebarScrollProps(RightSidebarScrollField.DataSource)}
size="s"
>
<h2>
{i18n.translate('workspace.creator.form.associateDataSourceTitle', {
defaultMessage: 'Associate data sources',
})}
</h2>
</EuiTitle>
<EuiText size="xs">
{i18n.translate('workspace.creator.form.associateDataSourceDescription', {
defaultMessage:
'Add at least one data source that will be available in the workspace. If a selected OpenSearch connection has related Direct Query connections, they will also be available in the workspace.',
})}
</h3>
</EuiTitle>
<EuiText size="xs">
{i18n.translate('workspace.creator.form.associateDataSourceDescription', {
defaultMessage:
'Add data sources that will be available in the workspace. If a selected OpenSearch connection has embedded Direct Query connection, they will also be available in the workspace.',
})}
</EuiText>
<SelectDataSourcePanel
onChange={setSelectedDataSourceConnections}
savedObjects={savedObjects}
assignedDataSourceConnections={formData.selectedDataSourceConnections}
data-test-subj={`workspaceForm-dataSourcePanel`}
showDataSourceManagement={true}
/>
</EuiText>
<SelectDataSourcePanel
Copy link
Contributor

@wanglam wanglam Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need more spacing between description and SelectDataSourcePanel?

Copy link
Contributor Author

@Kapian1234 Kapian1234 Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In SelectDataSourcePanel, there's already a <EuiSpacer size="m" />, and it would display only when the table exists

onChange={setSelectedDataSourceConnections}
savedObjects={savedObjects}
assignedDataSourceConnections={formData.selectedDataSourceConnections}
data-test-subj={`workspaceForm-dataSourcePanel`}
showDataSourceManagement={true}
/>
</EuiPanel>
<EuiSpacer size="s" />
<EuiSpacer size="s" />
</>
Expand Down
10 changes: 4 additions & 6 deletions src/plugins/workspace/public/components/workspace_form/select_data_source_panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const SelectDataSourcePanel = ({
data-test-subj="workspace-creator-dataSources-assign-button"
>
{i18n.translate('workspace.form.selectDataSourcePanel.addNew', {
defaultMessage: 'Add OpenSearch connections',
defaultMessage: 'Associate OpenSearch connections',
})}
</EuiSmallButton>
);
Expand All @@ -94,7 +94,7 @@ export const SelectDataSourcePanel = ({
data-test-subj="workspace-creator-dqc-assign-button"
>
{i18n.translate('workspace.form.selectDataSourcePanel.addNewDQCs', {
defaultMessage: 'Add direct query connections',
defaultMessage: 'Associate direct query connections',
})}
</EuiSmallButton>
);
Expand Down Expand Up @@ -130,10 +130,8 @@ export const SelectDataSourcePanel = ({
<EuiFlexItem grow={false}>{addDirectQueryConnectionsButton}</EuiFlexItem>
)}
</EuiFlexGroup>
<EuiSpacer size="m" />
<EuiFlexItem style={{ maxWidth: 768 }}>
{assignedDataSourceConnections.length > 0 && renderTableContent()}
</EuiFlexItem>
{assignedDataSourceConnections.length > 0 && <EuiSpacer size="m" />}
{assignedDataSourceConnections.length > 0 && renderTableContent()}
{modalVisible && chrome && (
<AssociationDataSourceModal
savedObjects={savedObjects}
Expand Down
3 changes: 1 addition & 2 deletions ...plugins/workspace/public/components/workspace_form/workspace_permission_setting_input.tsx
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kapian1234 I think this file should not be included in this PR. Can we remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
EuiFlexItem,
EuiButtonIcon,
EuiSuperSelect,
EuiSuperSelectOption,
EuiFieldText,
} from '@elastic/eui';
import { i18n } from '@osd/i18n';
Expand Down Expand Up @@ -175,7 +174,7 @@ export const WorkspacePermissionSettingInput = ({
placeholder={
type === WorkspacePermissionItemType.User
? i18n.translate('workspaceForm.permissionSetting.selectUser', {
defaultMessage: 'Enter user name or uer ID',
defaultMessage: 'Enter user name or email address',
SuZhou-Joe marked this conversation as resolved.
Show resolved Hide resolved
})
: i18n.translate('workspaceForm.permissionSetting.selectUserGroup', {
defaultMessage: 'Enter group name or group ID',
Expand Down