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

WIP: Feature/search options #157

Open
wants to merge 2 commits into
base: betaStaging
Choose a base branch
from
Open

Conversation

vwalsh
Copy link
Collaborator

@vwalsh vwalsh commented Dec 4, 2022

search bar now works in some basic ways. consider:

  • UX
  • styling
  • if navigation to parts of this app would be nice by searching for a page's title (aka fuzzy search match on 'My Transactions' would navigate to the Transactions page)
  • any other endpoints to include in search
  • hints, tips, or other feedback to the user

WIP until most of the above is resolved and confirmed this implementation is adequate.

… working as wanted, with some small ux and styling changes likely warranted.
@vwalsh vwalsh changed the base branch from master to beta December 4, 2022 01:12
@vwalsh vwalsh changed the base branch from beta to betaStaging December 4, 2022 01:12
Base automatically changed from betaStaging to beta December 6, 2022 14:58
Copy link
Collaborator

@outragedhuman outragedhuman left a comment

Choose a reason for hiding this comment

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

See comments, also:

  1. There seems to be a bug on the search box where it's always showing up as if its :hover is active? Like it has a white outline all the time.

url: string;
}

const placeHolderVals = [{ title: "test" }, { title: "hello" }];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm assuming placeholders are no longer needed?

);

useEffect(() => {
console.log("render");
Copy link
Collaborator

Choose a reason for hiding this comment

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

could be cleaned up

@@ -10,7 +10,7 @@ import JUPSettingsMenu from "components/JUPSettingsMenu";
const drawerWidth = 240;

// Placeholder values for the autocomplete
const placeHolderVals = ["test", "hello"];
const placeHolderVals = [{ title: "test" }, { title: "hello" }];
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar to above, can placeholdervals be removed?

@outragedhuman outragedhuman changed the base branch from beta to betaStaging February 14, 2023 17:36
@outragedhuman
Copy link
Collaborator

@vwalsh I made some comments on this a while back. I still need to do some more testing on this so let me know when you've had a chance to look at the comments.

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