Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Adding Support for Localization of Error Messages #1304

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 8 commits into
base: master
Choose a base branch
Loading
from
Open

Adding Support for Localization of Error Messages #1304

wants to merge 8 commits into from

Conversation

hatsu38
Copy link

@hatsu38 hatsu38 commented May 20, 2023 •
edited
Loading

What kind of change does this PR introduce?

  • Bug Fix
  • Feature
  • Refactoring
  • Style
  • Build
  • Chore
  • Documentation
  • CI

Did you add tests for your changes?

  • Yes, my code is well tested
  • Not relevant

If relevant, did you update the documentation?

  • Yes, I've updated the documentation
  • Not relevant

Summary
Explain the motivation for making this change. What existing problem does the pull request solve?
Try to link to an open issue for more information.

#1289
#1260

Does this PR introduce a breaking change?
If this PR introduces a breaking change, please describe the impact and a migration path for existing applications.

Other information

Updating limits　8 KB->30KB.

8:16:20 PM: ✔ Adding to empty webpack project
8:16:20 PM:   dist/index.js
8:16:20 PM:   Size limit: 25 kB
8:16:20 PM:   Size:       24.91 kB with all dependencies, minified and gzipped
8:16:20 PM:   dist/es/index.js
8:16:20 PM:   Package size limit has exceeded by 382 B
8:16:20 PM: Failed during stage 'Install dependencies': dependency_installation script returned non-zero exit code: 1
8:16:20 PM:   Size limit: 25 kB
8:16:20 PM:   Size:       25.38 kB with all dependencies, minified and gzipped

FYI: https://app.netlify.com/sites/react-dropzone/deploys/6468abcc0afc9f0008ab0b1a

The following image is a confirmation image that it is displayed in the local language on localhost
スクリーンショット 2023-05-20 19 59 33

スクリーンショット 2023-05-20 20 02 18 スクリーンショット 2023-05-20 19 57 13

@hatsu38 hatsu38 changed the title Feature/add local error msg Adding Support for Localization of Error Messages May 20, 2023
@hatsu38 hatsu38 marked this pull request as ready for review May 20, 2023 11:26
rolandjitsu
rolandjitsu reviewed Oct 2, 2024
View reviewed changes
Copy link
Collaborator

@rolandjitsu rolandjitsu left a comment

Choose a reason for hiding this comment

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

While I agree with the intent of the PR, the impl. could be a bit more generic.

How about we allow customizing all the messaging (harcoded strings) instead, then let the user do what they please with these messages (e.g. apply i18n).

This was referenced Oct 3, 2024
Open
Open
@hatsu38
Copy link
Author

hatsu38 commented Oct 5, 2024

@rolandjitsu

How about we allow customizing all the messaging (harcoded strings) instead, then let the user do what they please with these messages (e.g. apply i18n).

That might be a good idea!Can you give me some more specific steps?

@rolandjitsu
Copy link
Collaborator

rolandjitsu commented Oct 5, 2024 via email

@rolandjitsu
Copy link
Collaborator

I think a property that can be set on the hook could work. This could be a func, e.g. errMsg(key: string) string, that gets called to get the err msg for a given key, or some object with static msg definitions.
…
On Sat, Oct 5, 2024, 06:11 hatsu @.> wrote: @rolandjitsu https://github.com/rolandjitsu How about we allow customizing all the messaging (harcoded strings) instead, then let the user do what they please with these messages (e.g. apply i18n). That might be a good idea!Can you give me some more specific steps? — Reply to this email directly, view it on GitHub <#1304 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALJIOMR6XUMDYOE63CHM7DZZ5KHXAVCNFSM6AAAAABPH6Z5IWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJUHA3TSNBWHE . You are receiving this because you were mentioned.Message ID: @.>

@hatsu38 let me know if this makes sense.

@rolandjitsu rolandjitsu mentioned this pull request Oct 26, 2024
Open
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rolandjitsu rolandjitsu rolandjitsu left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hatsu38 @rolandjitsu
Morty Proxy This is a proxified and sanitized view of the page, visit original site.