search

wordpress-mobile / WordPress-iOS-Shared

Shared components used in building the WordPress iOS apps and other library components
GNU General Public License v2.0
18 stars 22 forks source link

Remove FormatterKit. #176

Closed yaelirub closed 5 years ago

yaelirub commented 5 years ago

As per @stevebaranski suggestion in p77Llu-bxU-p2#comment-12227 Also removed unused function toStringForPageSections.

instead of using the TTTTimeIntervalFormatter for dates that are less than a week ago we use our own relativeStringDate() This implementation does not include future dates formatting ("from now"). It seems to me like "from now" was not used in the app (looked at scheduled posts) let me know if I'm wrong and it is used and needed.

yaelirub commented 5 years ago

Thanks so much @stevebaranski . Investigating further and will update.

yaelirub commented 5 years ago

Hey @stevebaranski . I updated the PR. Please let me know what you think

ghost commented 5 years ago

@yaelirub I checked out the branch, and the relativeStringDate seems to be working in my manual tests. That said, I noticed that the ago suffix isn't localized like it was previously in/with FormatterKit.

This piqued my interest. So, I created a Swift playground to illustrate that you can accomplish most of what needs to be done with DateComponentsFormatter. While the class evinces locale-sensitive intervals, it does not appear to be capable of adding the "ago" suffix either, which FormatterKit refers to as "idiomatic deictic expressions".

If we resolve to remove FormatterKit and maintain parity for existing WPiOS callers of Date.mediumString(), I believe we'd need to do a few things:

  1. Remove this new relativeStringDate, as I don't think GlotPress is currently setup for this framework (I don't see existing localized strings in here).
  2. Revise the mediumString() method to behave like the playground; we might want two formatters based on whether we're looking at an interval greater than or less than 24 hours.
  3. In WPiOS, transition callers of this mediumString method to a new method that calls mediumString(), but also appends ago in a locale-sensitive way when the interval is in the past.
  4. Facilitate the addition of these localized strings.

Given this new information, do you think it's worthwhile to proceed with the effort as we described?

yaelirub commented 5 years ago

@stevebaranski . Thanks for the information. Honestly, I'm not sure anymore. Sounds like FormatterKit might have value still?Especially since DateComponentsFormatter doesn't provide relativity in dates. What do you think?

ghost commented 5 years ago

@yaelirub I'm okay with closing this. It might be worth creating an issue with our findings, in the event someone else would like to take it on.

Everything but the "ago" bit sees to be available via DateComponentsFormatter. Perhaps we could file a Radar to add it, in much the same way as "time remaining" is added via includesTimeRemainingPhrase.

I really appreciate you investigating this one!

yaelirub commented 5 years ago

Closing this. Opened issue #177