-
Notifications
You must be signed in to change notification settings - Fork 890
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
[Look&Feel] Refactor to use semantic headers for page, modal & flyout #7192
[Look&Feel] Refactor to use semantic headers for page, modal & flyout #7192
Conversation
❌ 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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7192 +/- ##
=======================================
Coverage 67.55% 67.55%
=======================================
Files 3469 3469
Lines 68479 68479
Branches 11130 11130
=======================================
Hits 46264 46264
Misses 19512 19512
Partials 2703 2703
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
❌ 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. |
ce3d66d
to
3d64d00
Compare
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.
Clarifications on guidance after going through these:
- Let's only make changes to page content headers or modal/flyout headers - other h1/h2 tags, let's ignore as they may not be being used correctly or are there for accessibility purposes.
- Let's swap out EuiTitle for EuiText size='s' (or wrap when not present)
- If its using a small or xs EuiTitle, let's probably not change as they're intentionally small.
Also, I think you will want to search for all modals/flyouts to adjust their headers. I don't think they all use header tags today so may not be coming up in your search.
src/plugins/dashboard/public/application/utils/get_no_items_message.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/application/components/loading_spinner/loading_spinner.tsx
Outdated
Show resolved
Hide resolved
src/plugins/opensearch_dashboards_overview/public/components/add_data/add_data.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/application/utils/get_no_items_message.tsx
Outdated
Show resolved
Hide resolved
src/plugins/vis_default_editor/public/components/sidebar/sidebar_title.tsx
Outdated
Show resolved
Hide resolved
src/plugins/visualizations/public/wizard/type_selection/type_selection.tsx
Outdated
Show resolved
Hide resolved
@@ -42,7 +42,7 @@ interface VisHelpTextProps { | |||
export const VisHelpText = ({ name, title, description, highlightMsg }: VisHelpTextProps) => { | |||
return ( | |||
<React.Fragment> | |||
<EuiTitle size="s"> | |||
<EuiTitle size="m"> |
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.
Is this in a visualization? If so, let's not change - given its intentionally small, not sure if we should change either way. If you can point of screenshot, would help me give feedback
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.
<h1 id="visualizeListingHeading"> | ||
<FormattedMessage | ||
id="visualize.listing.createNew.title" | ||
defaultMessage="Create your first visualization" |
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.
Let's leave this as is because its in the visualization
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.
8cedb84
to
60d41e4
Compare
Updated the PR to use EuiText accordingly and refactored missing modals/flyouts to use semantic headers. |
a3f7c80
to
cc9dc8b
Compare
Signed-off-by: Zhongnan Su <[email protected]>
cc9dc8b
to
1de9642
Compare
@virajsanghvi This is actually being reverted and will remain "xs". I will correct the screenshot |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-7192-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 96fb9d629e6539db4dacd922df3fff6809fdb407
# Push it to GitHub
git push --set-upstream origin backport/backport-7192-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x Then, create a pull request where the |
…#7192) * [Look&Feel] refactor to use semantic header for page, modal and flygout Signed-off-by: Zhongnan Su <[email protected]> * Changeset file for PR #7192 created/updated --------- Signed-off-by: Zhongnan Su <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 96fb9d6) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…#7192) (#7223) * [Look&Feel] refactor to use semantic header for page, modal and flygout * Changeset file for PR #7192 created/updated --------- (cherry picked from commit 96fb9d6) Signed-off-by: Zhongnan Su <[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: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
Refactor to use semantic headers for page, modal & flyout
Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration