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

Add MaskedInput component #189

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Add MaskedInput component #189

merged 1 commit into from
Nov 11, 2024

Conversation

stephannv
Copy link
Contributor

@stephannv stephannv commented Nov 11, 2024

@cirdes asked me to add a MaskedInput component, at first the plan was to use IMask, but the mask was configured using only JS and the conversion Ruby => Javascript didn't work well. So I suggested using Mask because it accepts mask config via data attributes, so we wouldn't need to convert anything from Ruby to JS.

I'm adding the examples to ruby-ui/web too:
Screenshot 2024-11-11 at 11 45 47

Screenshot 2024-11-11 at 11 47 02

module RubyUI
class MaskedInput < Base
def view_template
Input(type: "text", **attrs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@stephannv , I like the way you did it here. It's using the Input component that we already have, which makes MaskedInput compatible with our forms.

We will have to find a way to mark the dependencies so the component generator could be smart enough to eject all dependencies correctly. But we can do that later


def test_render
output = phlex_context do
RubyUI.MaskedInput(data: {maska: "#####-###"})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was wondering if we should try to make it more generic like using mask as the data attribute name. But doing so, we will have to map all data attributes expected by maska. It will be simpler if we explicitly tell the users that maska is doing the mask job and point them to the mask docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think we are tied to the js lib API anyway, it would be difficult to have a generic approach here since each mask lib have distinct config formats.

Choose a reason for hiding this comment

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

I agree with @cirdes. I think mask is a much better name for the attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@brandondrew yeah, me too, but we would need to rename all maska config attributes to use mask instead of maska (data-mask, data-mask-tokens, data-mask-tokens-replace, etc). This would create an unwanted abstraction + additional code that I'm not sure if it will bring any benefits.

Copy link
Collaborator

@SethHorsley SethHorsley Nov 13, 2024

Choose a reason for hiding this comment

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

I agree 100% with @stephannv . This seems odd at first but it makes it explicitly clear that something is going on and if they look into it it will show we are using maska which will make sense.

@cirdes cirdes merged commit 992944d into ruby-ui:main Nov 11, 2024
2 checks passed
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.

4 participants