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

Fix: update glossary terms table stylings and temporarily bypass failing drag-and-drop test #18653

Conversation

shrushti2000
Copy link
Contributor

This PR introduces following fixes,

Glossary Terms Table Styling:
Fixes layout and styling issues for the terms table to improve readability and alignment.

Drag-and-Drop Test Update:
Temporarily disables a failing drag-and-drop test of Glossary Terms table for further investigation and debugging.

Screen.Recording.2024-11-15.at.11.36.00.AM.1.mov
Screenshot 2024-11-15 at 11 13 29 AM

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Copy link
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 63%
63.56% (39269/61778) 40.15% (15545/38718) 42.24% (4665/11044)

Comment on lines 195 to 214
.select-all-checkbox {
font-size: 14px;
color: @grey-4;
height: 33px;
padding-left: 28px;
display: flex;
align-items: center;
width: 100%;
margin-bottom: 4px;
.ant-checkbox-inner {
display: flex;
align-items: center;
}
.select-all-dropdown-text {
margin-left: 8px;
margin-top: 10px;
}
}

.custom-glossary-dropdown-action-btn {
Copy link
Collaborator

Choose a reason for hiding this comment

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

would recommend to use less custom css and more use of AntD component props to achieve alignment and designs

Copy link
Contributor Author

@shrushti2000 shrushti2000 Nov 15, 2024

Choose a reason for hiding this comment

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

Done,have reduced the use of custom css wherever possible and replaced it with classnames

Comment on lines 222 to 224
.glossary-dropdown-actions-container {
padding-top: 5px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please do not use padding for aligenement fixes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, have removed the use of padding as an alignment fix

@@ -95,21 +95,19 @@
font-size: 16px;
color: @grey-3;
width: 100%;

margin-left: 15px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, have replaced this with spacing variable

Comment on lines 156 to 158
.ant-checkbox-wrapper {
height: 33px;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason to have custom height here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A custom height of 33px was added to .ant-checkbox-wrapper to visually align the checkbox with adjacent elements, as the default height caused minor misalignment.

Copy link

@Sachin-chaurasiya Sachin-chaurasiya merged commit 393c7a4 into open-metadata:main Nov 15, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test Add this label to run secure Github workflows on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants