-
Notifications
You must be signed in to change notification settings - Fork 885
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
refactor: density and consistency changes for discover and query bar #7299
refactor: density and consistency changes for discover and query bar #7299
Conversation
Signed-off-by: Viraj Sanghvi <virajs@amazon.com>
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
Signed-off-by: Viraj Sanghvi <virajs@amazon.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7299 +/- ##
=======================================
Coverage 67.50% 67.50%
=======================================
Files 3501 3501
Lines 69343 69343
Branches 11305 11305
=======================================
+ Hits 46808 46810 +2
+ Misses 19779 19777 -2
Partials 2756 2756
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Viraj Sanghvi <virajs@amazon.com>
Signed-off-by: Viraj Sanghvi <virajs@amazon.com>
@@ -110,15 +110,15 @@ function FilterBarUI(props: Props) { | |||
|
|||
const button = ( | |||
<EuiButtonEmpty | |||
size="xs" | |||
size="s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callout: increasing as it needs to match neighbor
@@ -108,7 +108,7 @@ class FilterEditorUI extends Component<Props, State> { | |||
public render() { | |||
return ( | |||
<div> | |||
<EuiPopoverTitle> | |||
<EuiPopoverTitle paddingSize="s"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callout: placing padding on popovertitle because there isn't a popover to place this on (css change handles the body)
@@ -34,7 +34,7 @@ | |||
// Unlike most inputs within layout control groups, the text area still needs a border. | |||
// These adjusts help it sit above the control groups shadow to line up correctly. | |||
padding: ($euiSizeS + 2px) $euiSizeS $euiSizeS; | |||
transform: translateY(-2px) translateX(-1px); | |||
transform: translateY(-1px) translateX(-1px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callout: this makes the focus state align properly like other inputs
@@ -52,7 +52,9 @@ export function DataGridFlyout({ | |||
<EuiFlyoutBody> | |||
<EuiFlexGroup direction="column"> | |||
<EuiFlexItem> | |||
<DocViewerLinks hit={hit} indexPattern={indexPattern} columns={columns} /> | |||
<EuiText size="s"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callout: placing here instead of inside component to avoid changing font size in expanded view
@@ -241,7 +241,7 @@ export const getTopNavLinks = ( | |||
}), | |||
run() { | |||
inspector.open(inspectorAdapters, { | |||
title: savedSearch?.title, | |||
title: savedSearch?.title || undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callout: defaulting to undefined just makes sure a title shows up in Inspector panel worst case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
i think for this to be all in one. ty so much |
…7299) * refactor: density and consistency changes for discover and query ba --------- Signed-off-by: Viraj Sanghvi <virajs@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 54cdf23) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…7299) (#7321) * refactor: density and consistency changes for discover and query ba --------- (cherry picked from commit 54cdf23) Signed-off-by: Viraj Sanghvi <virajs@amazon.com> 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: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
This does a number of things in Discover and Query Bar (let me know if I should split this up):
Note: most up to date OUI changes aren't present in mocks as OSD main doesn't use them yet.
Issues Resolved
N/A
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration