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

REPLAY-1448 Add support for UIStepper #1194

Merged
merged 8 commits into from
Mar 10, 2023

Conversation

maciejburda
Copy link
Member

@maciejburda maciejburda commented Mar 7, 2023

What and why?

📦 This PR enhances the recording of UIStepper elements in session replay. Its preliminary support was introduced earlier, but here we solve remaining glitches and send frames instead of images for button + and - button icons.

How?

By adding new recorder for UIStepper element that checks it's frame and creates matching wireframes.

testSteppers()-allowAll-privacy
testSteppers()-allowAll-privacy

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@maciejburda maciejburda force-pushed the maciey/REPLAY-1448-uistepper branch from 40eb7fc to 8bdba29 Compare March 7, 2023 11:46
@maciejburda maciejburda marked this pull request as ready for review March 8, 2023 12:28
@maciejburda maciejburda requested a review from a team as a code owner March 8, 2023 12:28
@maciejburda
Copy link
Member Author

One thing to note. Pixel perfection is not there - due to center calculation it's sometimes slightly offset. I tried mitigating with using text shape, but the results weren't as nice for font related reasons. The other solution is to use + and - images that are embedded within the control, but this means sending all the image data.

Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Nice 🚀 - I left 2 feedbacks to consider.

One thing to note. Pixel perfection is not there - due to center calculation it's sometimes slightly offset. I tried mitigating with using text shape, but the results weren't as nice for font related reasons. The other solution is to use + and - images that are embedded within the control, but this means sending all the image data.

We seek for an "approximated" look that will fulfil product requirements and what we did here is more than enough 👍. Only difference between what we do now vs we had considered is 5 vs 3-4 wireframes. This is IMO negligible (even when scrolling, sending updates to 5 wireframes is not much worse than updating 3 of them). Well done 💪.

PR fixes
@maciejburda maciejburda merged commit 9c78109 into develop Mar 10, 2023
@maciejburda maciejburda deleted the maciey/REPLAY-1448-uistepper branch March 10, 2023 11:01
@ncreated ncreated mentioned this pull request Mar 31, 2023
6 tasks
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.

2 participants