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.

Sign up for GitHub

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

Jump to bottom

feat(global-header): implemented support button in header #373

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat(global-header): implemented support button in header #373

wants to merge 16 commits into from
+1,888 −279

Conversation

ciiay
Copy link
Contributor

@ciiay ciiay commented Jan 31, 2025 •
edited
Loading

Hey, I just made a Pull Request!

For RHIDP-5252

  • support button implementation
  • notifications button implementation
  • removed hardcoded colors

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

screen recording(updated on Feb 6th):
Tested with RHDH PR #2263, and theme update PR

rhidp_5252_final.mp4

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Jan 31, 2025 •
edited
Loading

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
app workspaces/global-header/packages/app none v0.0.0
backend workspaces/global-header/packages/backend none v0.0.0
@red-hat-developer-hub/backstage-plugin-global-header workspaces/global-header/plugins/global-header major v0.0.4

christoph-jerolimov
christoph-jerolimov reviewed Feb 4, 2025
View reviewed changes
workspaces/global-header/plugins/global-header/dev/index.tsx Outdated
@@ -71,7 +71,7 @@ const defaultGlobalHeaderComponentsMountPoints: GlobalHeaderComponentMountPoint[
props: {
icon: 'support',
tooltip: 'Support',
to: '/support',
to: 'https://github.com/redhat-developer/rhdh/issues',
Copy link
Member

Choose a reason for hiding this comment

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

When you open that page and click on create you will notice that we redirect all our users to Jira instead.

So it should be:

Suggested change
to: 'https://github.com/redhat-developer/rhdh/issues',
to: 'https://issues.redhat.com/browse/RHIDP',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch on my dev code. Thank you 🤝

Hmm, https://issues.redhat.com/browse/RHIDP is an internal Jira page, do the users have the access to it?
https://github.com/redhat-developer/rhdh/issues is the one anyone can visit right?

Copy link
Member

Choose a reason for hiding this comment

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

But GH issues will be empty and users shouldn't ask questions there.

Since it's just a dev page you can maybe simple use https://developers.redhat.com/rhdh/overview ?

Copy link

Choose a reason for hiding this comment

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

This link goes directly to the create issue form if that is preferrable: https://issues.redhat.com/secure/CreateIssue.jspa?pid=12335748&issueType=13263

@ciiay ciiay force-pushed the rhidp-5252-complete-header-icon-buttons branch from 4ed7f09 to 557271b Compare February 5, 2025 04:25
@ciiay ciiay marked this pull request as ready for review February 5, 2025 04:36
@ciiay ciiay requested a review from ShiranHi February 5, 2025 04:42
@ShiranHi
Copy link

ShiranHi commented Feb 5, 2025

@ciiay it looks great!
Question: Can we update the tooltip text to "Support (external site)" when the link directs to an external page?

@ciiay ciiay force-pushed the rhidp-5252-complete-header-icon-buttons branch 3 times, most recently from 0374d2b to d2313e4 Compare February 7, 2025 22:11
@its-mitesh-kumar
Copy link
Contributor

@ciiay , Not able to see unit tests , will we be targeting it as part of another story ?

@ciiay ciiay force-pushed the rhidp-5252-complete-header-icon-buttons branch from 449d0e6 to 6de364c Compare February 10, 2025 21:57
@sonarqubecloud SonarQubeCloud
Copy link

Please retry analysis of this Pull-Request directly on SonarQube Cloud

ciiay added 9 commits February 10, 2025 17:19
@ciiay ciiay force-pushed the rhidp-5252-complete-header-icon-buttons branch from 0e36418 to 0774a82 Compare February 10, 2025 22:19
@ciiay ciiay mentioned this pull request Feb 11, 2025
Open
5 tasks
@ciiay
Copy link
Contributor Author

ciiay commented Feb 11, 2025

@ciiay , Not able to see unit tests , will we be targeting it as part of another story ?

Will work on a separate PR. This PR needs to get in and release the first version for RHDH cluster testing asap.

ciiay added 2 commits February 10, 2025 23:32
@ciiay
bug fix
db87451 
Signed-off-by: Yi Cai <[email protected]>
@ciiay
bug fix
a65f944 
Signed-off-by: Yi Cai <[email protected]>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@logonoff logonoff logonoff left review comments

@debsmita1 debsmita1 Awaiting requested review from debsmita1 debsmita1 is a code owner

@divyanshiGupta divyanshiGupta Awaiting requested review from divyanshiGupta divyanshiGupta is a code owner

@gashcrumb gashcrumb Awaiting requested review from gashcrumb gashcrumb is a code owner

@its-mitesh-kumar its-mitesh-kumar Awaiting requested review from its-mitesh-kumar its-mitesh-kumar is a code owner

@ShiranHi ShiranHi Awaiting requested review from ShiranHi

@christoph-jerolimov christoph-jerolimov Awaiting requested review from christoph-jerolimov christoph-jerolimov is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ciiay @ShiranHi @its-mitesh-kumar @christoph-jerolimov @logonoff