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.

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

Don't show rate limits if the reset time has passed. #356

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Don't show rate limits if the reset time has passed. #356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c6516e8
Don't show rate limits if the reset time has passed.
iamdharmesh Feb 1, 2025
900384d
Merge branch 'develop' into fix/rate-limits
dkotter Feb 5, 2025
6c07d36
Don't show App limit if reset time is already passed.
iamdharmesh Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions includes/core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,13 @@ function get_app_rate_limits_markup( $rate_limits ) {
* @return string
*/
function get_rate_limits_markup( $title, $remaining, $limit, $reset ) {
// If reset time is less than current time, then rate limit is not available.
if ( $reset && $reset < time() ) {
return sprintf(
'<p>%s</p>',
esc_html__( 'No X/Twitter rate limit available yet. Make a post to X/Twitter first.', 'autoshare-for-twitter' )
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as the message here goes, do we want to still refer to things as X/Twitter or should we start just using X? Not sure how we refer to things in other places right now

Copy link
Member Author

Choose a reason for hiding this comment

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

We are still using X/Twitter in most places, but I think X is now well-known enough that we can drop Twitter from all places. @jeffpaul, what do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For what it's worth, my opinion here is for any new strings we introduce, we should just use X and can look to update existing strings in a separate PR. I'm also fine though if we want to continue to use X/Twitter, just noting that reads a little awkwardly and the change has been in place long enough that I think anyone using this plugin would understand what just X means

Copy link
Member

Choose a reason for hiding this comment

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

Probably fine now to update to just "Autopost for X" and "X" throughout.

);
}

$remaining = isset( $remaining ) ? (int) $remaining : esc_html__( 'N/A', 'autoshare-for-twitter' );
$limit = isset( $limit ) ? (int) $limit : esc_html__( 'N/A', 'autoshare-for-twitter' );
Expand Down
6 changes: 5 additions & 1 deletion src/js/components/TwitterAccounts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,11 @@ function TwitterAccount( props ) {
* @return {JSX.Element} The account rate limits.
*/
function TwitterUserRateLimits( { rate_limits: rateLimits } ) {
if ( ! rateLimits || ! rateLimits.user_limit_24hour_limit ) {
if (
! rateLimits ||
! rateLimits.user_limit_24hour_limit ||
rateLimits?.user_limit_24hour_reset < Math.floor( Date.now() / 1000 )
) {
return (
<p>
{ __(
Expand Down
Loading