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

Disallow newlines in text filter input #4052

Open
seancolsen opened this issue Dec 6, 2024 · 3 comments · May be fixed by #4078
Open

Disallow newlines in text filter input #4052

seancolsen opened this issue Dec 6, 2024 · 3 comments · May be fixed by #4078
Assignees
Labels
beta: approved Temporary label to mark issues that are approved ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory

Comments

@seancolsen
Copy link
Contributor

Steps to reproduce

  1. Use our library sample data.
  2. Go to the Publishers table page.
  3. Filter by Name equals Crown (in title case).
  4. Expect and observe one result. Good.
  5. Edit the filter value and add a trailing newline after the text.
  6. Expect that the text input does not allow newlines. Expect nothing to change when pressing Enter.
  7. Instead, observe that a newline is inserted into the input, changing the filter condition. Observe that no records are found. Bad.
@seancolsen seancolsen added ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory labels Dec 6, 2024
@seancolsen seancolsen added this to the v0.2.0 (beta release) milestone Dec 6, 2024
@seancolsen seancolsen self-assigned this Dec 6, 2024
@mathemancer
Copy link
Contributor

Question: Why not give some obvious indicator when the field contains a newline? There are perfectly valid reasons to have a newline in a database field (for example addresses). With our current filtering logic, you'd often need to actually include a new line in the filter definition if you wanted to filter on some text that goes over the line break.

@seancolsen
Copy link
Contributor Author

@mathemancer

The goal of this issue is to improve product onboarding by reducing friction points that we've observed during user testing. This problem came up here (at 25:40). If you watch, you'll see that the user presses Enter out of instinct and then gets confused.

If we disallow newlines in text filter inputs, then yes, users will not be able to search using phrases containing newlines. I understand that. But I'm not convinced that's a problem worth addressing right now.

My prediction is that it would be exceedingly rare for a user to ever want to filter via a literal search phrase containing a newline. I've done text searches like this in the past, but only when using regular expressions. And in those cases I've specified the newlines via \n in my regex. In contrast, I expect it to be quite common for users to press Enter after typing their search value. Coincidentally, just today I had lunch with a techie friend and gave her a demo of Mathesar. I noticed that she pressed Enter while filtering too. She happened to be filtering a number, so she didn't run into this issue, but it did stand out to me.

I think there are two relatively straightforward approaches here:

  • Design the filtering input to prioritize power, by allowing it to search using phrases that contain newlines. (This is the current behavior.)
  • Design the filtering input to prioritize familiarity, by making the Enter key behave in a manner similar to other applications people have used. (This is the behavior I've proposed.)

In general I think it's worth trying to achieve power and familiarity, when feasible. But these goals are often in tension, as is the case here.

If we "give some obvious indicator when the field contains a newline" (as you say), then yes, that strikes somewhat of a balance, but I still think it places too much weight on power at the cost of usability. I'd rather optimize for the scenario that we expect to be more common. Personally I wouldn't press Enter here. But the user testing I've done has given me the expectation that it will be quite common.

Another idea would be to allow users to enter newlines via Shift+Enter and also show a tooltip on Enter which says: "Filtering is performed as you type. To enter a newline, use Shift+Enter". I would be fine with this design, but it's more work, and I'm not sure it's worth the implementation time right now.

In the grand scheme of filtering data, our current filter functionality is already so woefully lacking in power that I wouldn't expect it to meet users needs 100% of the time. Probably not even 75%. And that's okay. The good thing about our current filtering functionality is that it's simple and somewhat familiar. I would absolutely like to have something more powerful, but in order to prioritize power I'd rather give users a different feature altogether like SQL querying.

@mathemancer
Copy link
Contributor

Another idea would be to allow users to enter newlines via Shift+Enter and also show a tooltip on Enter which says: "Filtering is performed as you type. To enter a newline, use Shift+Enter". I would be fine with this design, but it's more work, and I'm not sure it's worth the implementation time right now.

I like this idea. Perhaps we can keep it in mind for the long run.

Overall, I see your point about tension vs. familiarity. I do think that if done properly, this is a spot where we can distinguish ourselves from the competition by offering more sophisticated filtering without resorting to the SQL text box.

Anywhoo, I don't want to hold up this issue. We can always do the modifier-Enter option later.

@kgodey kgodey added the beta: approved Temporary label to mark issues that are approved label Dec 11, 2024
@seancolsen seancolsen linked a pull request Dec 13, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta: approved Temporary label to mark issues that are approved ready Ready for implementation type: bug Something isn't working work: frontend Related to frontend code in the mathesar_ui directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants