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

Use same date format for next episode and previous #477

Merged
merged 5 commits into from
Dec 6, 2022

Conversation

TylerCarberry
Copy link
Contributor

@TylerCarberry TylerCarberry commented Oct 24, 2022

Description

Use the same date format for the predicted episode date and the dates in the list. I'm in the US, I think that's why the dates are in the different order

Before & After
Before After

@TylerCarberry TylerCarberry requested a review from a team as a code owner October 24, 2022 23:54
@@ -43,7 +43,7 @@ class RelativeDateFormatter(val context: Context) {
}
} else if (Locale.getDefault().language == "en") {
// if using an old Android version but using English then add Today and Yesterday
val alternativeString = formatCloseDaysOld(calendar, resources)
val alternativeString = formatCloseDaysOld(calendar, context.resources)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also removed this resources param since there's already a field for context

Copy link
Member

@geekygecko geekygecko left a comment

Choose a reason for hiding this comment

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

Thanks for looking making the next episode much nicer to read and cleaning up the context code. I think there is an issue with our RelativeDateFormatter not supporting future dates as November 6 isn't this Sunday but the next. Did you want to take a look at this? If not I'm happy to.

@TylerCarberry
Copy link
Contributor Author

Fixed! Just double check this is the format you want to show. I think it should be clear from the context whether "sunday" means last sunday (episode release date) or next sunday (prediction date)

@ashiagr ashiagr self-requested a review November 22, 2022 08:00
Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

Thank you so much for the changes, @TylerCarberry 🙏 and really sorry for taking so long to review.

I just left a couple of super minor suggestions. Other than that, date formatting looks much better now! 🎉

Also, it'll be awesome to add a CHANGELOG entry to 7.28 for the fix.

And you can run the following command to autocorrect spotless formatting issue:
./gradlew :spotlessApply

@TylerCarberry TylerCarberry requested a review from ashiagr December 6, 2022 02:04
Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

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

Thank you! 🙏

@ashiagr ashiagr merged commit 2e1b0bc into Automattic:main Dec 6, 2022
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