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

feat: improve currency type (append local aware curreny name in label) #174

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Chris53897
Copy link
Contributor

@Chris53897 Chris53897 commented Apr 12, 2024

#171

I am not sure what is the best way to implement it.
With this implementation it is an opt-in for the longer labels.

Default
before-only-code

With local currency name (german)
after-locale-name

Usage:
->add('currency', CurrencyType::class, ['label' => 'foo', 'currency_choices_label_format' => 1])

@Chris53897 Chris53897 changed the title feat: improve currency type feat: improve currency type (append local aware curreny name in label) Apr 12, 2024
@Chris53897 Chris53897 mentioned this pull request Apr 13, 2024
4 tasks
@johanwilfer
Copy link
Collaborator

Makes sense but maybe you can add constants and documentation for these new options as well? If possible a unit test as well?

@Chris53897
Copy link
Contributor Author

I can to. Maybe Enum instead of Constants. But maybe i will find a better way to implement it.
getLabel(...) that can be overwritten by the Users. I will need to have a look if this works and is easy to implement.

@johanwilfer
Copy link
Collaborator

Another idea is to use placeholders similar to how it works in translations maybe? And we can have:

  • %code% to be the currency code
  • %name% to be the currency name that you added

And then we can have a string instead to define the format/template to use, and it is very easy to extend maybe? (Default could then be %code% as that is the current default.

@johanwilfer
Copy link
Collaborator

Is this something you plan to work on @Chris53897 or should we close it?

@Chris53897
Copy link
Contributor Author

The earliest possible time for open source work is end of september for me.
I will try to have a look at your placeholder idea.

@johanwilfer
Copy link
Collaborator

Thanks @Chris53897 - will leave this open then :)

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.

3 participants