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

Add tooltip to help icon #3626

Merged
merged 2 commits into from
Apr 17, 2023
Merged

Conversation

sayuree
Copy link
Contributor

@sayuree sayuree commented Mar 17, 2023

Description

Added tooltip to a Help icon

Issues Resolved

fixes #3573

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@sayuree sayuree requested a review from a team as a code owner March 17, 2023 10:45
@ashwin-pc ashwin-pc added the OSCI Open Source Contributor Initiative label Mar 17, 2023
@@ -328,7 +328,7 @@ class HeaderHelpMenuUI extends Component<Props, State> {
})}
onClick={this.onMenuButtonClick}
>
<EuiIcon type="help" size="m" />
<EuiIcon type="help" size="m" title="Help" />
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<EuiIcon type="help" size="m" title="Help" />
<EuiIcon type="help" size="m" title={intl.formatMessage({
id: 'core.ui.chrome.headerGlobalNav.helpMenuButtonTitle',
defaultMessage: 'Help menu',
})} />

@KrooshalUX KrooshalUX self-requested a review March 17, 2023 21:14
Copy link

@KrooshalUX KrooshalUX left a comment

Choose a reason for hiding this comment

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

Please see comment in main issue: #3573 (comment)

@abbyhu2000 abbyhu2000 self-assigned this Mar 22, 2023
@abbyhu2000
Copy link
Member

Hi @sayuree, will you be able to make the suggested changes for this PR?

@sayuree
Copy link
Contributor Author

sayuree commented Mar 23, 2023

@abbyhu2000 yes, I am able to finish it.

size="m"
title={intl.formatMessage({
id: 'core.ui.chrome.headerGlobalNav.helpMenuButtonTitle',
defaultMessage: 'Help menu',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Help menu',
defaultMessage: 'Help',

I think the agreed upon text is simply "Help" - @KrooshalUX can you confirm?

Choose a reason for hiding this comment

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

It should be Help, yes.

@joshuarrrr
Copy link
Member

@sayuree This looks great! Can you also add an update to the CHANGELOG.md file? This can go under bug fixes category.

<EuiIcon
type="menu"
size="m"
title={i18n.translate('core.ui.primaryNav.toggleNavAriaLabel', {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
title={i18n.translate('core.ui.primaryNav.toggleNavAriaLabel', {
title={i18n.translate('core.ui.primaryNav.menu', {

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this should be core.ui.primaryNav.toggleNavAriaLabel

@abbyhu2000
Copy link
Member

abbyhu2000 commented Mar 24, 2023

@sayuree Can you also add UI screenshots to show the tooltips on both the help icon and menu icon? Thank you!

@sayuree
Copy link
Contributor Author

sayuree commented Mar 28, 2023

@sayuree Can you also add UI screenshots to show the tooltips on both the help icon and menu icon? Thank you!

@abbyhu2000 Here are the screenshots:
Screenshot from 2023-03-28 15-31-40
Screenshot from 2023-03-28 15-31-48

@sayuree sayuree requested review from abbyhu2000 and removed request for joshuarrrr March 28, 2023 09:34
@joshuarrrr
Copy link
Member

@KrooshalUX Screenshots provided.

KrooshalUX
KrooshalUX previously approved these changes Mar 28, 2023
Copy link

@KrooshalUX KrooshalUX left a comment

Choose a reason for hiding this comment

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

LGTM

@abbyhu2000
Copy link
Member

LGTM, will approve after the tests finish running

@joshuarrrr
Copy link
Member

@sayuree There are some snapshot tests that need to be updated: https://github.com/opensearch-project/OpenSearch-Dashboards/actions/runs/4532314219/jobs/8036242750?pr=3626

You can update snapshots via yarn run test:jest -u

Signed-off-by: sabina.zaripova <[email protected]>

Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>

Edit tooltip text

Signed-off-by: sabina.zaripova <[email protected]>

Add updates to CHANGELOG

Signed-off-by: sabina.zaripova <[email protected]>

Edit title

Signed-off-by: sabina.zaripova <[email protected]>

Update snapshots

Signed-off-by: sabina.zaripova <[email protected]>
Signed-off-by: sabina.zaripova <[email protected]>
@sayuree
Copy link
Contributor Author

sayuree commented Apr 16, 2023

@joshuarrrr @abbyhu2000 I have rebased the PR, removed unnecessarily updated snapshots. Tests should approve now.

@abbyhu2000 abbyhu2000 added v2.7.0 and removed v2.8.0 labels Apr 17, 2023
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

LGTM

@abbyhu2000 abbyhu2000 merged commit 8b0587e into opensearch-project:main Apr 17, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-1.x 1.x
# Navigate to the new working tree
pushd ../.worktrees/backport-1.x
# Create a new branch
git switch --create backport/backport-3626-to-1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8b0587e7aa683ebe9a25c4a454ddb117c68d7335
# Push it to GitHub
git push --set-upstream origin backport/backport-3626-to-1.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-1.x

Then, create a pull request where the base branch is 1.x and the compare/head branch is backport/backport-3626-to-1.x.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-1.3 1.3
# Navigate to the new working tree
pushd ../.worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-3626-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8b0587e7aa683ebe9a25c4a454ddb117c68d7335
# Push it to GitHub
git push --set-upstream origin backport/backport-3626-to-1.3
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-3626-to-1.3.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3626-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8b0587e7aa683ebe9a25c4a454ddb117c68d7335
# Push it to GitHub
git push --set-upstream origin backport/backport-3626-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3626-to-2.x.

abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Apr 17, 2023
* Add tooltip to help icon

Signed-off-by: sabina.zaripova <[email protected]>

Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Apr 17, 2023
* Add tooltip to help icon

Signed-off-by: sabina.zaripova <[email protected]>

Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Apr 17, 2023
* Add tooltip to help icon

Signed-off-by: sabina.zaripova <[email protected]>

Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
abbyhu2000 pushed a commit to abbyhu2000/OpenSearch-Dashboards that referenced this pull request Apr 17, 2023
* Add tooltip to help icon

Signed-off-by: sabina.zaripova <[email protected]>

Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
joshuarrrr pushed a commit that referenced this pull request Apr 18, 2023
* Add tooltip to help icon
* Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
Co-authored-by: sayuree <[email protected]>
ashwin-pc pushed a commit that referenced this pull request Apr 20, 2023
* Add tooltip to help icon



Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
Co-authored-by: sayuree <[email protected]>
ashwin-pc pushed a commit that referenced this pull request Apr 20, 2023
* Add tooltip to help icon



Add tooltip to menu icon

Signed-off-by: sabina.zaripova <[email protected]>
Co-authored-by: sayuree <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add clarifying tooltips to header navigation
7 participants