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

Feature Request: I18n cancel link #499

Closed
knoobie opened this issue Aug 11, 2019 · 8 comments · Fixed by #518
Closed

Feature Request: I18n cancel link #499

knoobie opened this issue Aug 11, 2019 · 8 comments · Fixed by #518

Comments

@knoobie
Copy link
Contributor

knoobie commented Aug 11, 2019

Currently the cancel link has a hardcoded english message and a × that is not changeable.

Proposal:

instead of:

  showCancelLink: true

how about:

  cancel: {
    enable: true,
    caption: '×', // can be html
    aria-label: 'aria-label is only added to the component if config is present and != null',
  }
@RobbieTheWagner
Copy link
Member

@knoobie I am open to making things configurable. Shouldn't everything be i18n capable then? Not sure what else we might need to tweak.

@knoobie
Copy link
Contributor Author

knoobie commented Aug 12, 2019

@rwwagner90 I'm a huge fan of i18n everywhere. Let me check later if I can find more. That was the first thing I found.

@knoobie
Copy link
Contributor Author

knoobie commented Aug 12, 2019

Update: I think all other things are i18n already :)

@RobbieTheWagner
Copy link
Member

@knoobie do you think we need to be able to change the "x"? I feel like we can keep that, and we could then add cancelLinkLabel as a new option or something.

@knoobie
Copy link
Contributor Author

knoobie commented Aug 12, 2019

Was wondering the same.. but for example - a company uses FontAwesome as icon base - they would probably wanna use <i class="far fa-times-circle"></i> as icon.

@RobbieTheWagner
Copy link
Member

Fair enough. I just don't want to have a breaking API change for this small improvement, so perhaps we should add cancelLinkLabel and cancelLinkHTML or something.

@knoobie
Copy link
Contributor Author

knoobie commented Aug 12, 2019

Fair enough!

@RobbieTheWagner
Copy link
Member

@knoobie I just merged a different breaking change, so we should be okay to do a breaking change here. Will get something up.

RobbieTheWagner added a commit that referenced this issue Aug 17, 2019
This change allows us to pass multiple options for cancelIcon, so we can do things like set the label.

Closes #499
RobbieTheWagner added a commit that referenced this issue Aug 17, 2019
* showCancelLink -> cancelIcon

This change allows us to pass multiple options for cancelIcon, so we can do things like set the label.

Closes #499

* Fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants