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 format.timeFormatter to get raw access to the formatter #31

Merged

Conversation

cibernox
Copy link
Contributor

The reason behind this PR is that I need to call formatter.formatToParts instead of simply formatter.format.

I'm happy to change the approach if you think some other approach is more flexible while allowing to call formatToParts.

If this approach seems ok I can add the same method for the dateFormatter too.

Fixes #30

@kaisermann
Copy link
Owner

Hey! Thanks for the PR.

What do you think about exporting the getXxxxxFormatter methods?

// index.ts
export {
  getDateFormatter,
  getNumberFormatter,
  getTimeFormatter,
  getMessageFormatter,
} from './includes/formatters'

Then you would be able to pass a locale and all the other parameters you want.

@cibernox
Copy link
Contributor Author

@kaisermann I'm ok with that, but unless I'm mistaken if the user changes the selected locale, the app wouldn't be notified the same way they are when using store, right?

For completenes, this is a store I've created that returns a function that knows how to format the time of the next high/low tide

import { t } from "svelte-i18n";

export const formatNextTideDate = derived([t], ([format]) => {
  return (nextTideDate, centerDate) => {
    let centerDateInSeconds = centerDate.getTime() / 1000;
    let nextTideDateInSeconds = nextTideDate.getTime() / 1000;
    if (
      centerDateInSeconds >= nextTideDateInSeconds - FIFTEEN_MINUTES_IN_SECONDS &&
      centerDateInSeconds <= nextTideDateInSeconds + FIFTEEN_MINUTES_IN_SECONDS
    ) {
      return format("tide.now");
    } else {
      return format.time(nextTideDate, { format: 'short' });
    }
  }
});

@kaisermann
Copy link
Owner

Yeah, being a low-level API, you would need to keep the locale updated. Following your example, it's simple to do that, just also derive from the locale store:

import { t, locale } from '...'

export const customFormatThingie = derived([t, locale], (format, currentLocale) => ....);

Would that help you?

@cibernox
Copy link
Contributor Author

@kaisermann yeah, that makes sense. I've updated the PR to export the getXxxxxFormatters from the index. I've updated test/client/includes/formatters.test.ts to import the tested utilities from the index, so we're also asserting not only that they work but that they are exposed publicly.

@cibernox cibernox force-pushed the add-method-to-get-raw-formatter branch from be6e92e to 4bd82fe Compare December 27, 2019 22:15
@cibernox
Copy link
Contributor Author

Any feedback on this one that I should address?

@cibernox
Copy link
Contributor Author

cibernox commented Jan 4, 2020

ping

@kaisermann
Copy link
Owner

Hey, @cibernox, I'm going to take a look at this PR's code soon. Been busy with the beginning of the year 😁

@kaisermann kaisermann merged commit 4fc3a96 into kaisermann:master Jan 7, 2020
kaisermann pushed a commit that referenced this pull request Jan 7, 2020
* Add test
* Change approach, export functions to access the formatters instead
@kaisermann
Copy link
Owner

Released on v2.2.0. Added some documentation to the wiki: https://github.com/kaisermann/svelte-i18n/wiki/Methods#low-level-api

@cibernox
Copy link
Contributor Author

cibernox commented Jan 7, 2020

Great, thanks!

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.

Allow to call formatToParts on a formatter
2 participants