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

Search functionality without HOC #390

Closed
schanzer opened this issue Sep 7, 2021 · 7 comments
Closed

Search functionality without HOC #390

schanzer opened this issue Sep 7, 2021 · 7 comments
Assignees
Labels
code quality Issues that involve improving code quality but don't necessarily contribute to features/bug fixes

Comments

@schanzer
Copy link
Member

schanzer commented Sep 7, 2021

We currently wrap our BlockEditor in a higher order component that provides additional functionality. This is unnecessary.

@pcardune
Copy link
Collaborator

pcardune commented Sep 7, 2021

I figured out the right keystrokes to open the search window and it looks kind of janky to me. Is this functionality needed at all? If so I'm tempted to just rewrite it.

@schanzer
Copy link
Member Author

schanzer commented Sep 8, 2021

No objection! Just bear in mind that there's some accessibility stuff embedded into react-tabs and react-modal that we don't want to lose.

@pcardune
Copy link
Collaborator

pcardune commented Sep 8, 2021

Before I embark on the effort, is there a doc or something that explains the goals of the search functionality? For example, I see that it supports regex search. If the primary use case for the editor is to help beginners learn about programming, regex search seems like a pretty advanced feature that makes the interface more complicated.

Is there any reason the search functionality needs to look or behave any differently than codemirror's search functionality, aside from highlighting blocks in addition to text?

@schanzer
Copy link
Member Author

schanzer commented Sep 8, 2021

@sorawee added regexp functionality because he could, and I certainly wasn't going to argue with free functionality! :)

You're right, though - this is more than we need. There isn't any formal requirements doc, but here's the checklist in descending order of priority:

  • UI should work well with screenreaders (happy to go into more detail if you like, but the current audio UI largely passes muster and could be copied)
  • Case-insensitive search-by-string
  • Search-by-block
  • Distinct sounds for "no match found" and "wrapping to next match" (these are already implemented)
  • Case-sensitive search-by-string (likely a checkbox in the same UI for case-insensitive)
  • Use PageUp/PageDown keys for "find next" and "find previous" (this is from long discussions with the a11y consultant, and the fact that so many kids use chromebooks)

@pcardune
Copy link
Collaborator

pcardune commented Sep 8, 2021

Thanks for the checklist, this is helpful.

@sorawee added regexp functionality because he could, and I certainly wasn't going to argue with free functionality! :)

Nothing is free. All features have a maintenance cost for developers and a learning/onboarding cost for users. Like everything else, these costs have to be balanced against the benefits. As they say, "less is more", except of course when more is more. As much as I am wowed by the lambda calculus, it sure is nice to have things that aren't functions.

@schanzer
Copy link
Member Author

schanzer commented Sep 9, 2021

image

@pcardune pcardune added the code quality Issues that involve improving code quality but don't necessarily contribute to features/bug fixes label Oct 14, 2021
@pcardune
Copy link
Collaborator

I'm going to close this in favor of #485

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Issues that involve improving code quality but don't necessarily contribute to features/bug fixes
Projects
None yet
Development

No branches or pull requests

2 participants