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

[Multiple datasource] Adjust the padding size for aggregated view #6715

Merged
merged 28 commits into from
May 17, 2024
Merged

[Multiple datasource] Adjust the padding size for aggregated view #6715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e3a8a21
adjust the aggregated view padding
yujin-emma Apr 25, 2024
b51db9e
WIP
yujin-emma Apr 26, 2024
f701566
Update src/plugins/data_source_management/public/components/data_sour…
yujin-emma May 3, 2024
555acd5
Changeset file for PR #6715 created/updated
opensearch-changeset-bot[bot] May 7, 2024
1b09d37
Update CHANGELOG.md
yujin-emma May 7, 2024
e8776fe
[saved objects] enable deletion of saved objects by type if configure…
kavilla May 5, 2024
ab37eb2
[tests] fix tests related to #6443 (#6722)
kavilla May 6, 2024
1bbabe2
feat: add config for topRightNavigation (#6712)
raintygao May 7, 2024
620d852
move the css to the data source aggregated view
yujin-emma May 7, 2024
077298f
[Multiple Datasource Test]add more test for icon and aggregated view …
yujin-emma May 7, 2024
d435cca
update css file space and use the oui color guideline number to repla…
yujin-emma May 8, 2024
e4b74a1
[OSD Availability] Prevent OSD process crashes when disk is full (#6733)
Flyingliuhub May 8, 2024
49ae21f
[MDS] Modify toast + popover warning to include incompatible datasour…
huyaboo May 8, 2024
d091efe
[Multiple Datasource Test] Add test for edit data source form (#6742)
yujin-emma May 9, 2024
cae2a3f
[MQL] support enhancing language selector (#6613)
kavilla May 9, 2024
bead9ae
Make Field Name Search Filter Case Insensitive (#6759)
LDrago27 May 10, 2024
e8715b3
[Multiple Datasource Test] Add test for error_menu, item, data_source…
yujin-emma May 10, 2024
1beabfb
[2.14][chore] update release notes (#6705)
kavilla May 11, 2024
18d1b1d
add http://www.site.com to lycheeignore (#6771)
yujin-emma May 13, 2024
1f68fea
[Workspace] Fix: Show a error toast when workspace read only user del…
yubonluo May 14, 2024
297eaa5
Updating security reachout email (#6778)
varun-lodaya May 14, 2024
57ca1ab
add @zhyuanqi as a maintainer (#6788)
BionIT May 14, 2024
9560ca0
[OE] Adds dev doc script to precommit hook (#6585)
ashwin-pc May 15, 2024
9971d4f
Move @BSFishy to emeritus maintainer (#6790)
BionIT May 15, 2024
f50addd
add @mengweieric as maintainer (#6798)
BionIT May 15, 2024
f7130d2
Add OpenAPI specification for get and create saved object APIs (#6799)
BionIT May 15, 2024
09d09a0
[Multiple Datasource Test]Add test for toast button and validation fo…
yujin-emma May 16, 2024
0891292
Merge branch 'main' into list-padding
yujin-emma May 16, 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/6715.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Adjust the padding size for aggregated view ([#6715](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6715))
26 changes: 25 additions & 1 deletion ...management/public/components/data_source_aggregated_view/data_source_aggregated_view.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@

.euiSelectableListItem__content {
cursor: default;
padding-left: 0;
margin-right: 0;


.euiSelectableListItem__text {
max-height: 100%;
max-height: 100%;
}
.euiSelectableListItem__icon, .euiSelectableListItem__prepend {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be an extra line between .euiSelectableListItem__icon, and .euiSelectableListItem__text

margin-right: 0;
}
}

Expand All @@ -31,6 +36,7 @@
overflow: hidden;
white-space: nowrap;
display: inline-block;
padding-left: 0;
}
}

Expand All @@ -39,4 +45,22 @@
align-items: center;
justify-content: center;
}

.dataSourceAggregatedViewOuiFlexItem {
color: grey;
Copy link
Member

Choose a reason for hiding this comment

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

should we use the oui color guide?

Copy link
Contributor Author

@yujin-emma yujin-emma May 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to refer to the guides here: https://oui.opensearch.org/1.6/#/guidelines/colors and updated to 5B6875 which is grey from the guideline, also confirmed from the local, but the code commit does not allow it: Using the value "#5b6875" is not allowed. All colors should be inherited from the OUI theme, not hard-coded. Either remove the custom color rule or use OUI SASS variables instead
Therefore, still use grey

text-overflow: ellipsis;
overflow: hidden;
white-space: nowrap;
display: inline-block;
padding-left: 0;
}

.dataSourceListAllActiveOuiFlexItem {
color: grey;
Copy link
Member

Choose a reason for hiding this comment

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

same as above

text-overflow: ellipsis;
overflow: hidden;
white-space: nowrap;
display: inline-block;
padding-left: 1em;
}
}
6 changes: 5 additions & 1 deletion ..._management/public/components/data_source_aggregated_view/data_source_aggregated_view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,11 @@ export class DataSourceAggregatedView extends React.Component<
options={items}
renderOption={(option) => (
<DataSourceItem
className={'dataSourceAggregatedView'}
className={
this.props.displayAllCompatibleDataSources
? 'dataSourceAggregatedView'
Copy link
Member

Choose a reason for hiding this comment

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

just checking due to naming. but it seems like on line 242 if !this.props.displayAllCompatibleDataSources then it will render the dataSourceAggregatedViewOuiPanel

but in this logic its this.props.displayAllCompatibleDataSources adds the dataSourceAggregatedView style. Is that accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, Rocky, I confirm the logic here is when :
displayAllCompatibleDataSources -> true -> list all ( code: https://github.com/yujin-emma/OpenSearch-Dashboards/blob/main/examples/multiple_data_source_examples/public/components/data_source_list_all_example.tsx#L104 )
displayAllCompatibleDataSources -> false -> list active ( code: https://github.com/yujin-emma/OpenSearch-Dashboards/blob/main/examples/multiple_data_source_examples/public/components/data_source_list_active_example.tsx#L111 )

: 'dataSourceListAllActive'
}
option={option}
defaultDataSource={this.state.defaultDataSource}
/>
Expand Down
Loading