Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Jan 26, 2023

Fixes #19995

Description

Total cards show "higher" word instead of "lower" label. The problem happened because 2 labels had the same identifiers.

Testing instructions

  1. Launch the JP and login.
  2. Navigate to "My Site → Stats".
  3. Ensure the Total cards are on your screen. If not, add them via the ⚙️ button at the top of the screen.
  4. Verify that positive changes should say ... higher than, negative changes should say ... lower than.

Regression Notes

  1. Potential unintended areas of impact

none

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

none

  1. What automated tests I added (or what prevented me from doing so)

none

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Images & Videos

Before

before_bug_higher_lower

After

after_bug_higher_lower

@staskus
Copy link
Contributor Author

staskus commented Jan 26, 2023

@mokagio

Letting you know that I'm aiming to merge it the fix to 21.6. It's a tiny issue but the fix will not affect anything else.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 26, 2023

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr20000-d5f57f1 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 26, 2023

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr20000-d5f57f1 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

irfano
irfano previously approved these changes Jan 26, 2023
Copy link
Member

@irfano irfano 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 the quick fix! Both negative and positive texts are correct now!

@staskus staskus changed the base branch from trunk to release/21.6 January 26, 2023 09:19
@staskus staskus dismissed irfano’s stale review January 26, 2023 09:19

The base branch was changed.

@staskus staskus force-pushed the fix/19995-total-cards-show-higher-word-instead-of-lower branch from 5251a21 to d5f57f1 Compare January 26, 2023 09:47
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Thanks @staskus! Code change looks good :D and I see @irfano tested it.

@staskus staskus merged commit fa20a3f into release/21.6 Jan 26, 2023
@staskus staskus deleted the fix/19995-total-cards-show-higher-word-instead-of-lower branch January 26, 2023 14:00
@mokagio
Copy link
Contributor

mokagio commented Jan 27, 2023

@staskus thanks for the ping.

Just FYI, this will necessitate a manual re-generation of the strings file which will then be sent to GlotPress and notify translators about the new string. That's all to say, it might take a while before seeing translated versions of this string in the betas.

I thought we had automation in place to detect duplicated keys, it'll be interesting to investigate how this made it past our checks. Most likely answer is that my mental model is incorrect.

Finally, congrats on opening PR n. 20,000 👏

mokagio added a commit that referenced this pull request Jan 27, 2023
This is a followup to the changes from fa20a3f

See discussion at
#20000 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Total cards show "higher" word instead of "lower"

6 participants