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

Jump to bottom

Improve Product Review Block design. #1681

Merged
merged 8 commits into from
Jul 10, 2023
Merged

Improve Product Review Block design. #1681

merged 8 commits into from
Jul 10, 2023

Conversation

Soare-Robert-Daniel
Copy link
Contributor

@Soare-Robert-Daniel Soare-Robert-Daniel commented May 30, 2023 •
edited
Loading

Closes #1312 .

Summary

Improve Product Review block design by allowing to be more compact.

Thanks to @mghenciu for the feedback.

Screenshots

One Column with Inline active

image

Two Column with Inline active

image

2023-05-30.15-26-10.mp4

Test instructions

  1. Make sure that nothing is breaking.
  2. New Inline style does not break the layout

Checklist before the final review

  • Visual elements are not affected by independent changes.
  • It is at least compatible with the minimum WordPress version.
  • It loads additional script in frontend only if it is required.
  • Does not impact the Core Web Vitals.
  • In case of deprecation, old blocks are safely migrated.
  • It is usable in Widgets and FSE.
  • Copy/Paste is working if the attributes are modified.
  • PR is following the best practices

@Soare-Robert-Daniel Soare-Robert-Daniel marked this pull request as ready for review May 30, 2023 12:53
@pirate-bot
Copy link
Contributor

pirate-bot commented May 30, 2023 •
edited
Loading

Bundle Size Diff

Package Old Size New Size Diff
Animations 192.39 KB 192.39 KB 0 B (0.00%)
Blocks 1.34 MB 1.34 MB 592 B (0.04%)
CSS 6.9 KB 6.9 KB 0 B (0.00%)
Dashboard 44.32 KB 44.32 KB 0 B (0.00%)
Export Import 4.74 KB 4.74 KB 0 B (0.00%)
Pro 223.74 KB 223.74 KB 0 B (0.00%)

@pirate-bot
Copy link
Contributor

pirate-bot commented May 30, 2023 •
edited
Loading

Plugin build for a600c56 is ready 🛎️!

@pirate-bot
Copy link
Contributor

pirate-bot commented May 30, 2023 •
edited
Loading

E2E Summary

Typing

Test Average Time (ms) Standard Deviation (ms) Median Time (ms) Quantile for soft limit (%) Quantile for hard limit (%)
Typing 64.11 21.21 58.08 84.21 (60ms) 89.47 (80ms)
Values above 60ms "4 - 137.26, 5 - 60.34, 9 - 111.54"

@Soare-Robert-Daniel Soare-Robert-Daniel linked an issue May 31, 2023 that may be closed by this pull request
Making use of Finmasters' Review Design #1312
Closed
@HardeepAsrani
Copy link
Member

Here I think instead of having a new toggle, we can just use Block Styles for the Inline option, and just keep it one column when it's set to inline. What do you think @JohnPixle?

HardeepAsrani
HardeepAsrani requested changes May 31, 2023
View reviewed changes
Copy link
Member

@HardeepAsrani HardeepAsrani left a comment

Choose a reason for hiding this comment

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

Awaiting confirmation from John.

JohnPixle
JohnPixle approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@JohnPixle JohnPixle left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I tested it and it works fine, thanks Robert!
@HardeepAsrani regarding what you said about the toggle, perhaps we can look into it in a future release, as I have spotted a couple of other opportunities for improvement as well.

For this release I'd say it's good to go, unless of course you have any objections!

@rodica-andronache
Copy link

@Soare-Robert-Daniel
One Column with Inline rating enabled - on mobile looks broken https://vertis.d.pr/i/fTz3Ne compared to how it looks when Inline rating is not enabled https://vertis.d.pr/i/p15ev4

Same happens for Two columns

@Soare-Robert-Daniel Soare-Robert-Daniel self-assigned this Jun 14, 2023
@pirate-bot pirate-bot added the pr-checklist-complete The Pull Request checklist is complete. (automatic label) label Jun 15, 2023
@Soare-Robert-Daniel
Copy link
Contributor Author

@Codeinwp/design-team, should we also add the following style with the number under the start from inline to the base one?

This is with the new feature.

image

This is the original

image

@JohnPixle
Copy link
Contributor

should we also add the following style with the number under the start from inline to the base one?

@Soare-Robert-Daniel yes, thanks for the heads-up

@Soare-Robert-Daniel
Copy link
Contributor Author

@JohnPixle, how does this look?

The order:

  • Inline One Column Style
  • Inline Two Column Style
  • Default One Column Style
  • Default Two Column Style
2023-06-19.15-48-03.mp4

@HardeepAsrani HardeepAsrani removed their request for review June 27, 2023 07:52
JohnPixle
JohnPixle approved these changes Jun 27, 2023
View reviewed changes
Copy link
Contributor

@JohnPixle JohnPixle left a comment

Choose a reason for hiding this comment

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

@Soare-Robert-Daniel I tested this on an instawp site and it works good, thanks a lot!

@JohnPixle
Copy link
Contributor

@Soare-Robert-Daniel just one thing, which might not be necessarily related to this, I got the following error when I installed the build. I used the development build.

Screenshot 2023-06-27 at 12 09 54 PM

Might be something worth seeing?

Here's a magic login for the instance.

@Soare-Robert-Daniel
Copy link
Contributor Author

@JohnPixle, thanks for the feedback.

Also, I will check the reported issue 🙏

@Soare-Robert-Daniel
Copy link
Contributor Author

@JohnPixle, in your instance, you have two versions of Otter installed simultaneously.

image

You should disable one of them.

Also, I double-checked on Wp Taste that the build is ok.

@JohnPixle
Copy link
Contributor

@Soare-Robert-Daniel Well, one instance of Otter is never enough for me!!
Thanks!

@rodica-andronache
Copy link

@Soare-Robert-Daniel everything's working well 👍

@HardeepAsrani HardeepAsrani merged commit d7d0718 into development Jul 10, 2023
@HardeepAsrani HardeepAsrani deleted the feat/product-review-inline branch July 10, 2023 10:56
@pirate-bot
Copy link
Contributor

🎉 This PR is included in version 2.3.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnPixle JohnPixle JohnPixle approved these changes

@HardeepAsrani HardeepAsrani HardeepAsrani approved these changes

Assignees

@Soare-Robert-Daniel Soare-Robert-Daniel

Labels
pr-checklist-complete The Pull Request checklist is complete. (automatic label) released Indicate that an issue has been resolved and released in a particular version of the product.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Making use of Finmasters' Review Design
5 participants
@Soare-Robert-Daniel @pirate-bot @HardeepAsrani @rodica-andronache @JohnPixle