#5616 remove duplicated scrollbars on dashboard #6990

l0rdn1kk0n · 2024-06-10T18:54:03Z

Description

Fixes an issue with duplicated scrollbars on dashboard.

I would highly recommend to refactor the mix and match between flex box and fixed sizes via styles as well as the div-hierarchy because it leads to hardly maintainable css and html.

Issues Resolved

closes #5616

Changelog

fix: Hide duplicated scrollbars in dashboard.

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
- tested in chrome and firefox manually
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

github-actions · 2024-06-10T18:54:33Z

ℹ️ Manual Changeset Creation Reminder

Please ensure manual commit for changeset file 6990.yml under folder changelogs/fragments to complete this PR.

If you want to use the available OpenSearch Changeset Bot App to avoid manual creation of changeset file you can install it in your forked repository following this link.

For more information about formatting of changeset files, please visit OpenSearch Auto Changeset and Release Notes Tool.

github-actions · 2024-06-10T19:00:58Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". 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.

Signed-off-by: Michael Haitz <[email protected]>

Signed-off-by: Zhongnan Su <[email protected]> Signed-off-by: Michael Haitz <[email protected]>

* Add changelog Signed-off-by: abbyhu2000 <[email protected]> * add another changelog Signed-off-by: abbyhu2000 <[email protected]> --------- Signed-off-by: abbyhu2000 <[email protected]> Signed-off-by: Michael Haitz <[email protected]>

Signed-off-by: Zhongnan Su <[email protected]> Signed-off-by: Michael Haitz <[email protected]>

* Add Suchit to be a maintainer Signed-off-by: abbyhu2000 <[email protected]> * Changeset file for PR opensearch-project#6980 created/updated * restore maintainer doc format Signed-off-by: abbyhu2000 <[email protected]> --------- Signed-off-by: abbyhu2000 <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Signed-off-by: Michael Haitz <[email protected]>

github-actions · 2024-06-11T11:25:15Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". 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.

github-actions · 2024-06-11T11:25:51Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "fix". 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.

virajsanghvi

Could you make the hiding of scrollbars more generic for the use cases where it doesn't make sense to scroll?

virajsanghvi · 2024-06-11T16:35:51Z

changelogs/fragments/6990.yml

@@ -0,0 +1,2 @@
+fix:
+- Hide duplicated scrollbars. ([#5616](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5616))


nit: think this needs to be indented to fix the workflow failure.

The issue is in table vis. We use embeddable to add table vis in Dashboard. Therefore, update table vis should limit the impact to local. Table vis is in src/plugins/vis_type_table.

virajsanghvi · 2024-06-11T16:37:46Z

src/plugins/visualizations/public/components/_visualization.scss

@@ -11,7 +11,7 @@
  flex-direction: column;
  width: 100%;
  height: 100%;
-  overflow: auto;
+  overflow: hidden;


I think this is too generic and likely needs to be more targeted. Some visualizations require scrolling to be able to access content when resized (e.g. text or controls)

changelogs/fragments/6990.yml

AMoo-Miki · 2024-06-11T21:09:16Z

src/plugins/visualizations/public/components/_visualization.scss

@@ -11,7 +11,7 @@
  flex-direction: column;
  width: 100%;
  height: 100%;
-  overflow: auto;
+  overflow: hidden;
  position: relative;
  padding: $euiSizeS;


The reason we get the extra scrollbars is that .visualization has a padding of $euiSizeS = 8px while its child has a margin of - $euiSizeL / 2 = -12px. This makes the child larger than the parent, forcing the scrollbars to show up. I would recommend you revert back to overflow: auto and make this change instead:

Suggested change

padding: $euiSizeS;

padding: calc($euiSizeL / 2);

ananzh · 2024-06-12T01:12:33Z

@l0rdn1kk0n Seems the changelog is updated from commits like 28e1c7c, not automatically generate from the bot.

To save you some future efforts, you could install https://github.com/apps/opensearch-changeset-bot then include Changelog in the description will trigger a changelog automatically.

Co-authored-by: Miki <[email protected]> Signed-off-by: l0rdn1kk0n <[email protected]>

ananzh · 2024-07-29T21:17:20Z

@l0rdn1kk0n do you have time to work on the fix? I updated it to 2.17

sejli · 2024-08-30T22:43:52Z

@l0rdn1kk0n going to push this back to 2.18 since 2.17 release is upcoming.

ananzh · 2024-10-30T19:19:12Z

@l0rdn1kk0n thanks for contributing in OSD. I will convert it to draft since no progress. Feel free to re-open once you fix the comments.

github-actions bot added the first-time-contributor label Jun 10, 2024

github-actions bot added the failed changeset label Jun 10, 2024

l0rdn1kk0n force-pushed the 5616-fix branch from 31e7b92 to bebfc33 Compare June 11, 2024 11:12

l0rdn1kk0n and others added 7 commits June 11, 2024 13:13

opensearch-project#5616 fix for duplicated scrollbars

0b79aef

Signed-off-by: Michael Haitz <[email protected]>

add changelog

28e1c7c

Signed-off-by: Michael Haitz <[email protected]>

update changelog

86f4c8e

Signed-off-by: Michael Haitz <[email protected]>

Add missing change log for MDS PRs (opensearch-project#6986)

c8b196a

Signed-off-by: Zhongnan Su <[email protected]> Signed-off-by: Michael Haitz <[email protected]>

Add missing change log for PR 6872, 6903 (opensearch-project#6994)

4201532

Signed-off-by: Zhongnan Su <[email protected]> Signed-off-by: Michael Haitz <[email protected]>

l0rdn1kk0n force-pushed the 5616-fix branch from bebfc33 to 2fc0e65 Compare June 11, 2024 11:13

Merge branch 'main' into 5616-fix

f77fa94

l0rdn1kk0n marked this pull request as ready for review June 11, 2024 11:24

l0rdn1kk0n requested review from ananzh, kavilla, AMoo-Miki, ashwin-pc, joshuarrrr, abbyhu2000, zengyan-amazon, zhongnansu, manasvinibs, ZilongX, Flyingliuhub, curq, bandinib-amzn, SuZhou-Joe and ruanyl as code owners June 11, 2024 11:24

l0rdn1kk0n requested review from BionIT, xinruiba, zhyuanqi and mengweieric as code owners June 11, 2024 11:24

virajsanghvi requested changes Jun 11, 2024

View reviewed changes

bandinib-amzn assigned bandinib-amzn and AMoo-Miki Jun 11, 2024

AMoo-Miki reviewed Jun 11, 2024

View reviewed changes

changelogs/fragments/6990.yml Outdated Show resolved Hide resolved

AMoo-Miki requested changes Jun 11, 2024

View reviewed changes

AMoo-Miki added backport 2.x v2.16.0 labels Jun 11, 2024

Update changelogs/fragments/6990.yml

77e567e

Co-authored-by: Miki <[email protected]> Signed-off-by: l0rdn1kk0n <[email protected]>

ananzh added v2.17.0 and removed v2.16.0 labels Jul 29, 2024

sejli removed the v2.17.0 label Aug 30, 2024

ananzh marked this pull request as draft October 30, 2024 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#5616 remove duplicated scrollbars on dashboard #6990

#5616 remove duplicated scrollbars on dashboard #6990

l0rdn1kk0n commented Jun 10, 2024 •

edited

Loading

github-actions bot commented Jun 10, 2024

github-actions bot commented Jun 10, 2024

github-actions bot commented Jun 11, 2024

github-actions bot commented Jun 11, 2024

virajsanghvi left a comment

virajsanghvi Jun 11, 2024

ananzh Jun 12, 2024

ananzh Jun 12, 2024

virajsanghvi Jun 11, 2024

AMoo-Miki Jun 11, 2024

ananzh commented Jun 12, 2024

ananzh commented Jul 29, 2024

sejli commented Aug 30, 2024

ananzh commented Oct 30, 2024

		@@ -0,0 +1,2 @@
		fix:
		- Hide duplicated scrollbars. ([#5616](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5616))

#5616 remove duplicated scrollbars on dashboard #6990

Are you sure you want to change the base?

#5616 remove duplicated scrollbars on dashboard #6990

Conversation

l0rdn1kk0n commented Jun 10, 2024 • edited Loading

Description

Issues Resolved

Changelog

Check List

github-actions bot commented Jun 10, 2024

ℹ️ Manual Changeset Creation Reminder

github-actions bot commented Jun 10, 2024

❌ Invalid Prefix For Manual Changeset Creation

github-actions bot commented Jun 11, 2024

❌ Invalid Prefix For Manual Changeset Creation

github-actions bot commented Jun 11, 2024

❌ Invalid Prefix For Manual Changeset Creation

virajsanghvi left a comment

Choose a reason for hiding this comment

virajsanghvi Jun 11, 2024

Choose a reason for hiding this comment

ananzh Jun 12, 2024

Choose a reason for hiding this comment

ananzh Jun 12, 2024

Choose a reason for hiding this comment

virajsanghvi Jun 11, 2024

Choose a reason for hiding this comment

AMoo-Miki Jun 11, 2024

Choose a reason for hiding this comment

ananzh commented Jun 12, 2024

ananzh commented Jul 29, 2024

sejli commented Aug 30, 2024

ananzh commented Oct 30, 2024

l0rdn1kk0n commented Jun 10, 2024 •

edited

Loading