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

feat(global-header): implemented searchBar #244

Merged

Conversation

ciiay
Copy link
Contributor

@ciiay ciiay commented Jan 7, 2025

Hey, I just made a Pull Request!

For RHIDP-5365

✔️ 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

rhidp_5365.mp4

@rhdh-gh-app
Copy link

rhdh-gh-app bot commented Jan 7, 2025

Changed Packages

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

@ciiay ciiay requested a review from ShiranHi January 7, 2025 04:40
Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

It looks really great!

Some small suggestions:

),
}}
sx={{
pt: '0.5rem',
Copy link
Member

Choose a reason for hiding this comment

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

For me, it feels like this padding-top breaks that the input field is vertically centered.

Suggested change
pt: '0.5rem',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this padding top, it looks closer to the top instead of centered virtually.

image

With pt
image

What do you think? Maybe I can make it pt: '6px' which looks like
image

Copy link
Member

Choose a reason for hiding this comment

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

To be honest, for me, the first one looks the best. 😏

@ciiay ciiay force-pushed the rhidp-5365-create-search-bar branch from fd0df0a to f910b22 Compare January 7, 2025 16:22
Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

Great work. 👍

@christoph-jerolimov christoph-jerolimov merged commit 2ab926b into redhat-developer:main Jan 8, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants