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

How does filter return error string to user? #49

Closed
rouilj opened this issue Jul 17, 2024 · 15 comments
Closed

How does filter return error string to user? #49

rouilj opened this issue Jul 17, 2024 · 15 comments

Comments

@rouilj
Copy link

rouilj commented Jul 17, 2024

I have a filter and I return "" to prevent the current string from being used as a tag. How do I notify the
user of the issue? Do I create a <span role="status"></span> after the input and set the innerText in
the filter?

@jcubic
Copy link
Owner

jcubic commented Jul 17, 2024

Filtering out tags is not an error. But you can create a span if you want.

@jcubic
Copy link
Owner

jcubic commented Jul 17, 2024

If you have particular API in mind on how the error/warning should be added that will be fully customized you can contribute with this feature. But first try to create a demo how it would work.

@rouilj
Copy link
Author

rouilj commented Jul 17, 2024

Filtering out tags is not an error. But you can create a span if you want.

While true, it leaves the user with an input that is not turning into a tag when they hit return. So it looks
like the tagger isn't working. So it's bad UI at the very least.

@jcubic
Copy link
Owner

jcubic commented Jul 17, 2024

I'm all ears if you have a proposal on how it should work.

@rouilj
Copy link
Author

rouilj commented Jul 21, 2024

As I see it three things have to happen:

  1. tagger needs to accept/handle an error from filter().
  2. the user needs to provide a place for the error message to be reported.
  3. tagger needs to be able to find #2 so it can put the error message from #1 into it. Also it
    needs to empty #2 if there was no error.

Error Implementation 1:

filter returns string or object

"IamATag", { "error": "Tag must start with a capital letter".}

Value of the error attribute in object is used as error message. I am not sure if that should be used
innerText/textContent or innerHtml. A this point I think innerText/textContent to reduce security
surface. In the future a type property "html|text" could be added to insert error as innerHtml.

Error Implemementation 2

filter returns string or throws() an Error

caller of filter puts filter call in a try/catch/finally/else block and filter calls:

  throw new Error("Tag must start with a capital letter")

and the message of the exception is the error. Other considerations same as in implementation 1.

User inserts html to contain the error mesage. Probably written like:

<div role="alert" id="tagger-propname-error"></div>

Tagger will need a "filter_error" parameter for tagger that accepts string|node. If string, find the node using document.querySelector (assumes filter_error is either an id, or a unique selector otherwise the first match will be used.). If node, just use it directly.

Tagger than inserts the error text using a_node.innerText/textContent.

Thoughts?

@jcubic
Copy link
Owner

jcubic commented Jul 21, 2024

Thanks for the suggestion. I think that it's way overcomplicated. I would just add:

tagger.error('error message');

method that the user needs to call in order to show the error:

filter(str) {
   if (invalid.includes(str)) {
       tagger.error('Invalid tag');
       return false;
   }
   tagger.error();
   return str;
}

@rouilj
Copy link
Author

rouilj commented Jul 22, 2024

How does tagger know where to put the error message? A second argument to tagger.error?

@jcubic
Copy link
Owner

jcubic commented Jul 22, 2024

What do you mean? There will be one place for error messages. I've never heard of components that have more than one place for errors.

@rouilj
Copy link
Author

rouilj commented Jul 22, 2024

Oh you are planning on adding your own error display location? I was expecting that to be under the control of the user so they can place it below, above, next to the control and style it as they want. It would probably be good to add a class to the component when there is an error so the user
can style the component correctly.

@jcubic
Copy link
Owner

jcubic commented Jul 23, 2024

If you have an idea how to implement tagger::error() and add a container for the error next to the input and so it remains in the same position when tagger is initialized, you can create a PR. If not, I will need to wait a bit when I will have time to work on this.

Also note that changing position of two elements with CSS is relatively easy.

@jcubic
Copy link
Owner

jcubic commented Jul 23, 2024

I think that the best solution is using this structure:

<div class="tagger wrap">
  <div class="tagger-container">
    <!-- the rest of the component -->
  <div>
  <div class="tagger-error">
    Invalid Tag
  </div>
</div>

and the tagger class can be a flexbox. With it, you can change the position of the error with flex-direction.

And while we at it, we can change the wrap class into tagger-wrap, so there are no conflict with other CSS on the page.

@jcubic
Copy link
Owner

jcubic commented Jul 23, 2024

Take a look at this proposal: (type a tag with f word).

https://codepen.io/jcubic/pen/WNqxPjY?editors=0110

In CSS panel, you can use commented out CSS to make the error above the component.

@rouilj
Copy link
Author

rouilj commented Jul 23, 2024 via email

@jcubic
Copy link
Owner

jcubic commented Jul 23, 2024

Please create a separate issue for each question or concern, so we can handle each issue individually.

If the error part is ok for you, I can commit the changes. I used Browser dev tools to overwrite and copy/paste the result into Codepen, it was faster to test this way.

@jcubic
Copy link
Owner

jcubic commented Aug 5, 2024

Ok, if you don't gonna test if the update works and not gonna create issue for features you want. I'm gonna close this one.

@jcubic jcubic closed this as completed Aug 5, 2024
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

No branches or pull requests

2 participants