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.

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

Fit and Finish Changes for DataStreams and Rollups #1153

Merged
merged 10 commits into from
Sep 2, 2024

Conversation

skumawat2025
Copy link
Contributor

@skumawat2025 skumawat2025 commented Aug 28, 2024

Description

Making fit and finish changes for DataStreams, Rollups and Notification settings pages according to the guidance

The change mostly include.

  1. Setting up the correct text size of heading according to guide.
  2. Using EuiPanel for the even padding across content.

Video:

Screenshare.-.2024-08-29.3_03_27.PM.mp4

Issues Resolved

[List any issues this PR will resolve]

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@skumawat2025 skumawat2025 force-pushed the fit-and-finish branch 2 times, most recently from 2ee5c11 to fba0e6e Compare August 28, 2024 10:30
@skumawat2025 skumawat2025 marked this pull request as ready for review August 28, 2024 10:30
@RamakrishnaChilaka
Copy link
Collaborator

@skumawat2025 can you please add details to the PR description.

@danieldong51
Copy link

Not sure if in scope, but the context menu here (public/pages/DataStreams/containers/DataStreamsActions/index.tsx) should have no padding:
Data streams context menu

@danieldong51
Copy link

The text here should also be wrapped in <EuiText size="s"> in public/pages/Rollups/components/RollupEmptyPrompt/RollupEmptyPrompt.tsx
Rollup jobs empty

@danieldong51
Copy link

danieldong51 commented Aug 28, 2024

The modal header here should be <h2> wrapped in <EuiText size="s">:

public/pages/RollupDetails/containers/RollupDetails/RollupDetails.tsx
Screenshot 2024-08-28 at 11 29 15 AM

@danieldong51
Copy link

The modal text here should be <EuiText size="s">:

public/pages/RollupDetails/containers/RollupDetails/RollupDetails.tsx
Screenshot 2024-08-28 at 11 31 52 AM

@RamakrishnaChilaka
Copy link
Collaborator

@danieldong51 Thanks a lot for reviewing this PR. @skumawat2025 let's work with Dan to get his approval on this PR.

@astute-decipher
Copy link
Contributor

LGTM, reviewed the pages offline.

SuZhou-Joe
SuZhou-Joe previously approved these changes Aug 29, 2024
Copy link
Member

@SuZhou-Joe SuZhou-Joe left a comment

Choose a reason for hiding this comment

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

Will merge it on 08/30 Beijing time if no more feedback.

@danieldong51
Copy link

I found a few more small changes, but everything else looked good!

Context menu in

  • public/pages/Rollups/containers/Rollups/Rollups.tsx should be size="s"
Screenshot 2024-08-29 at 12 08 29 PM

The titles when using the old UI for the following pages should be <EuiText size="s"> <h1> {title} <h1> <EuiText size="s"> (not sure if these were intentional):

  • public/pages/EditRollup/containers/EditRollup.tsx
  • The four create rollup steps in public/pages/CreateRollup/containers
  • public/pages/Notifications/containers/Notifications/Notifications.tsx
  • public/pages/ForceMerge/container/ForceMerge/ForceMerge.tsx
  • public/pages/Rollover/containers/Rollover/Rollover.tsx

@skumawat2025
Copy link
Contributor Author

skumawat2025 commented Sep 2, 2024

I found a few more small changes, but everything else looked good!

Context menu in

  • public/pages/Rollups/containers/Rollups/Rollups.tsx should be size="s"
Screenshot 2024-08-29 at 12 08 29 PM

The titles when using the old UI for the following pages should be <EuiText size="s"> <h1> {title} <h1> <EuiText size="s"> (not sure if these were intentional):

  • public/pages/EditRollup/containers/EditRollup.tsx
  • The four create rollup steps in public/pages/CreateRollup/containers
  • public/pages/Notifications/containers/Notifications/Notifications.tsx
  • public/pages/ForceMerge/container/ForceMerge/ForceMerge.tsx
  • public/pages/Rollover/containers/Rollover/Rollover.tsx

Thanks for looking into this PR. Have made these above changes in the latest commit.

Sandeep Kumawat added 5 commits September 2, 2024 14:07
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Sandeep Kumawat added 5 commits September 2, 2024 14:07
Signed-off-by: Sandeep Kumawat <[email protected]>
1. Remove padding from view Json Modal of rollup details page
2. Make steps smaller for create rollup page.
3. Remove extra padding from bottom buttons of rollup page

Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
@SuZhou-Joe SuZhou-Joe merged commit a7efceb into opensearch-project:main Sep 2, 2024
10 of 11 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 2, 2024
* Fit and Finish Changes for DataStreams and Rollups

Signed-off-by: Sandeep Kumawat <[email protected]>

* Makde changes for Rollup page

Signed-off-by: Sandeep Kumawat <[email protected]>

* Notification page changes

Signed-off-by: Sandeep Kumawat <[email protected]>

* Address PR comments

Signed-off-by: Sandeep Kumawat <[email protected]>

* Review comments

Signed-off-by: Sandeep Kumawat <[email protected]>

* Address offline review comments

Signed-off-by: Sandeep Kumawat <[email protected]>

* This commit includes-

1. Remove padding from view Json Modal of rollup details page
2. Make steps smaller for create rollup page.
3. Remove extra padding from bottom buttons of rollup page

Signed-off-by: Sandeep Kumawat <[email protected]>

* Old UI title changes

Signed-off-by: Sandeep Kumawat <[email protected]>

* make gaps between bottom button small

Signed-off-by: Sandeep Kumawat <[email protected]>

* resolve conflicts

Signed-off-by: Sandeep Kumawat <[email protected]>

---------

Signed-off-by: Sandeep Kumawat <[email protected]>
Co-authored-by: Sandeep Kumawat <[email protected]>
(cherry picked from commit a7efceb)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
SuZhou-Joe added a commit that referenced this pull request Sep 3, 2024
* Fit and Finish Changes for DataStreams and Rollups



* Makde changes for Rollup page



* Notification page changes



* Address PR comments



* Review comments



* Address offline review comments



* This commit includes-

1. Remove padding from view Json Modal of rollup details page
2. Make steps smaller for create rollup page.
3. Remove extra padding from bottom buttons of rollup page



* Old UI title changes



* make gaps between bottom button small



* resolve conflicts



---------



(cherry picked from commit a7efceb)

Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Sandeep Kumawat <[email protected]>
Co-authored-by: SuZhou-Joe <[email protected]>
@skumawat2025 skumawat2025 deleted the fit-and-finish branch September 4, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants