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

Allow search pin by name and cid #2046

Merged
merged 15 commits into from
Mar 31, 2022
Merged

Allow search pin by name and cid #2046

merged 15 commits into from
Mar 31, 2022

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Mar 25, 2022

closes #2024

search.mp4

@render
Copy link

render bot commented Mar 25, 2022

@render
Copy link

render bot commented Mar 25, 2022

@render
Copy link

render bot commented Mar 25, 2022

@github-actions github-actions bot added the Type: Feature Added to PRs to identify that the change is a new feature. label Mar 25, 2022
@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 25, 2022

added a small fix as we go, to avoid an overlap with the search icon (not visible in the video above)
image

@asnaith
Copy link
Member

asnaith commented Mar 25, 2022

Looks great in the video, will check it out once the api is ready 😊

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 30, 2022

Hey all, this has been updated, feel free to review

@asnaith
Copy link
Member

asnaith commented Mar 30, 2022

This is working great. I just found two small issues

  • The search input does not accept a space character so you'd never actually be able to search for "cute cat" 😿

  • The search filter stays applied when you navigate away (but the search input is cleared). This could be confusing if the last search yielded no results (see video).

Screen.Recording.2022-03-30.at.3.40.47.PM.mov

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 31, 2022

Thanks Andrew for the laser eyes, it's fixed now

@tanmoyAtb
Copy link
Contributor

Thanks Andrew for the laser eyes, it's fixed now

Just checked that the "name" field in Add pin also doesn't allow spaces in the name.

Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Looks good,

Just found the space issue in the ADD PIN modal, the current implementation looks great.

@asnaith
Copy link
Member

asnaith commented Mar 31, 2022

Thanks Andrew for the laser eyes, it's fixed now

Just checked that the "name" field in Add pin also doesn't allow spaces in the name.

@tanmoyAtb We have that fixed in another branch :) #2048

@Tbaut
Copy link
Collaborator Author

Tbaut commented Mar 31, 2022

ah that's where I have it. Well I fixed it here as well 21232f7

@Tbaut Tbaut merged commit 9629a59 into dev Mar 31, 2022
@Tbaut Tbaut deleted the feat/tbaut-search-cid-name-2024 branch March 31, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow search/filtering by pin name or cid
5 participants