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: add responsive search functionality with sticky search bar and … #587

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

Conversation

Ryadav0654
Copy link

Title:

feat: add responsive search functionality with sticky search bar and navigation

Linked Issue(s)

fixes: #556

Acceptance Criteria fulfillment

  • Sticky search bar remains at the top of the page as logs are scrolled.
  • Search input field allows searching through logs with matching terms highlighted.
  • Previous and Next buttons allow navigation through highlighted search results.
  • The layout of the search bar and buttons is responsive and adapts to mobile screens.

Proposed changes (including videos or screenshots)

  • Implemented a sticky search bar and navigation buttons that remain visible while scrolling through system logs.
  • Added functionality to highlight matching logs based on search input.
  • Integrated Previous and Next buttons to navigate through the highlighted search results.
  • Applied responsive design: On smaller screens, the search bar and buttons stack vertically for better usability.
  • Created a new search-functionality.css file for styling the search and navigation components, ensuring responsiveness with media queries.

Screenshot 2024-10-11 024944
Screenshot 2024-10-11 025009

Further comments

This change was made to improve the log searching experience within the application, eliminating the need to rely on browser-native search functionality. The sticky search bar ensures the search and navigation controls are always available, while responsive behavior enhances mobile usability.

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2024

CLA assistant check
All committers have signed the CLA.

@Ryadav0654
Copy link
Author

Hi @jayantbh ,

I've created this pull request that implements a sticky, responsive search bar with log navigation (Previous/Next) and highlighting of matching search terms. It includes mobile-friendly layout adjustments.

Please review and let me know your feedback!

Thanks!

@jayantbh
Copy link
Contributor

Hey, great PR!
Would be great if you could share a small screen recording showing this in action. :)

Additionally, going purely by the screenshot, I think we can use a bit softer colours. Even simply setting some opacity would help.

Plus, adding some icons beside Previous/Next buttons could help too.

If nothing is being searched, what happens then?

@jayantbh
Copy link
Contributor

image What are these characters at the start?


import { FlexBox } from '@/components/FlexBox';
import { Line } from '@/components/Text';
import { ServiceNames } from '@/constants/service';
import { useSelector } from '@/store';
import { TextField, Button } from '@mui/material';
import './search-functionality.css';
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is technically supported, this isn't generally how styles are defined and used in this codebase. :)

Usually it's some kind of a CSS-in-JS approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see this was resolved, but I'm not sure what was done to resolve it.

@jayantbh
Copy link
Contributor

Note: This isn't a complete review. That'll come from the team.

@Ryadav0654
Copy link
Author

Hey, great PR! Would be great if you could share a small screen recording showing this in action. :)

Additionally, going purely by the screenshot, I think we can use a bit softer colours. Even simply setting some opacity would help.

Plus, adding some icons beside Previous/Next buttons could help too.

If nothing is being searched, what happens then?

Recording.2024-10-11.203130.2.mp4

@jayantbh
Copy link
Contributor

You can hide the buttons if nothing is being searched, or nothing has matched the search term, of if there's only one match.

I'm also not seeing much of a change in the colors, though I notice a code change for that.

The icons are also disproportionately larger than the text they are beside, they should be a little smaller to fit in better.

And finally, there's seemingly no spacing between the divider and the first line of the logs.
image

@Ryadav0654
Copy link
Author

You can hide the buttons if nothing is being searched, or nothing has matched the search term, of if there's only one match.

I'm also not seeing much of a change in the colors, though I notice a code change for that.

The icons are also disproportionately larger than the text they are beside, they should be a little smaller to fit in better.

And finally, there's seemingly no spacing between the divider and the first line of the logs. image

Could you let me know which specific color you'd like to use or if you have any preferences?

@jayantbh
Copy link
Contributor

I think your chosen colors are fine, but I'm thinking 30% and 50% opacity for selected/unselected search matches.

@Ryadav0654
Copy link
Author

Ryadav0654 commented Oct 13, 2024

I think your chosen colors are fine, but I'm thinking 30% and 50% opacity for selected/unselected search matches.

Hi @jayantbh,

I've fixed the color opacity and hidden the button when no search results are found. Please review the changes.
Thanks!

Recording.2024-10-13.234631.1.mp4

@Ryadav0654 Ryadav0654 closed this Oct 13, 2024
@Ryadav0654 Ryadav0654 reopened this Oct 13, 2024
@jayantbh
Copy link
Contributor

image I think the yellow is fine to show selected line/segment.

But the white is a little to bright. Let's make its opacity 0.1 or 0.2 or something.

I really thought I already left this review comment, but I guess I didn't. 🤷🏽

@jayantbh
Copy link
Contributor

Hey @Ryadav0654 could you rebase your branch on main?
There's been some significant changes to how logs are rendered.

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.

Search functionality in the system logs UI
3 participants