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

[4.4] Fix display of total votes #43740

Merged
merged 4 commits into from
Jul 10, 2024
Merged

Conversation

Quy
Copy link
Contributor

@Quy Quy commented Jul 3, 2024

Pull Request for Issue #43728. Regression of #42934.

Summary of Changes

Move the number of votes outside of the visually-hidden section so it can be visible.

Testing Instructions

Enable Voting in Articles component configuration.
Vote for one or more articles.

Edit Content -Vote plugin.
Toggle Number of Votes to Show.

Actual result BEFORE applying this Pull Request

Total number of votes is not visible.

Expected result AFTER applying this Pull Request

Total number of votes is visible.

Copy link
Contributor

@chmst chmst left a comment

Choose a reason for hiding this comment

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

The whole block after <?php if ($rcount) : ?>needs to be indented

@ChristineWk
Copy link

I have tested this item ✅ successfully on 9ed4867


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43740.

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 9ed4867


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43740.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43740.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 4, 2024
@brianteeman
Copy link
Contributor

The whole block after <?php if ($rcount) : ?>needs to be indented

How can this be rtc while this is still outstanding to be fixed

@richard67
Copy link
Member

The whole block after <?php if ($rcount) : ?>needs to be indented

How can this be rtc while this is still outstanding to be fixed

To me the code style seems to be ok, and PHPCS seems to be ok with it, too.

@sandewt
Copy link
Contributor

sandewt commented Jul 6, 2024

Thanks for this PR and testing.

Just a question. Who or what ensures that this PR also applies to J5?

@richard67
Copy link
Member

Thanks for this PR and testing.

Just a question. Who or what ensures that this PR also applies to J5?

@sandewt Same as always for all bug fixes in 4.4-dev: When the PR has been merged into 4.4-dev, release managers of 5.1 will merge it up into 5.1-dev and so on.

@brianteeman
Copy link
Contributor

The whole block after <?php if ($rcount) : ?>needs to be indented

How can this be rtc while this is still outstanding to be fixed

To me the code style seems to be ok, and PHPCS seems to be ok with it, too.

when you look at the entire file (not just the snippet shown in github) then you can see that @chmst is correct

@richard67
Copy link
Member

@Quy Thanks for fixing the CS.

Leaving RTC status as the change after setting RTC was just a code style change, so previous human tests are still valid.

@chmst Thanks for review, and thanks @ChristineWk and @viocassel for testing.

@MacJoom MacJoom added this to the Joomla! 4.4.7 milestone Jul 10, 2024
@MacJoom MacJoom self-assigned this Jul 10, 2024
@MacJoom MacJoom merged commit 25168d3 into joomla:4.4-dev Jul 10, 2024
3 checks passed
@MacJoom
Copy link
Contributor

MacJoom commented Jul 10, 2024

Thank you!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 10, 2024
@Quy Quy deleted the 43728-show-total-votes branch July 10, 2024 20:02
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.

9 participants