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

Expose constant SKIP_TIME_THRESHOLD as skipTimeThreshold in replayer #1408

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Expose constant SKIP_TIME_THRESHOLD as skipTimeThreshold in replayer #1408

merged 2 commits into from
Apr 11, 2024

Conversation

avillegasn
Copy link
Contributor

There is this hardcoded constant SKIP_TIME_THRESHOLD to define the amount of time in milliseconds of an inactive period. By default it is set to 10 seconds, but I believe this should be exposed as a configuration option of the replayer so that users may configure it to fit their needs.

This pull request exposes SKIP_TIME_THRESHOLD as skipTimeThreshold.

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 12, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 5cc2515

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

eoghanmurray
eoghanmurray reviewed Apr 5, 2024
View reviewed changes
guide.md Outdated
@@ -301,6 +301,7 @@ The replayer accepts options as its constructor's second parameter, and it has t
| root | document.body | the root element of replayer |
| loadTimeout | 0 | timeout of loading remote style sheet |
| skipInactive | false | whether to skip inactive time |
| skipTimeThreshold | 10000 | a threshold of inactive time in milliseconds |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| skipTimeThreshold | 10000 | a threshold of inactive time in milliseconds |
| skipTimeThreshold | 10000 | the threshold in milliseconds for what should be considered an inactive period |

@eoghanmurray
Copy link
Contributor

Thanks for your contribution! I don't use the svelte player but this looks good.

May I suggest a better name for the config setting?

Maybe minInactivePeriod or inactivePeriodThreshold rather than skipTimeThreshold?

@avillegasn
Copy link
Contributor Author

Thanks for your contribution! I don't use the svelte player but this looks good.

May I suggest a better name for the config setting?

Maybe minInactivePeriod or inactivePeriodThreshold rather than skipTimeThreshold?

I did the change to inactivePeriodThreshold.

@eoghanmurray eoghanmurray merged commit 729361e into rrweb-io:master Apr 11, 2024
5 of 6 checks passed
@avillegasn avillegasn deleted the expose-skip-time-threshold branch April 11, 2024 10:20
eoghanmurray added a commit to eoghanmurray/rrweb that referenced this pull request Apr 17, 2024
@eoghanmurray
I forgot to pay attention to yarn format during merge of rrweb-io#1408
e3c57f1
eoghanmurray added a commit that referenced this pull request Apr 17, 2024
@eoghanmurray
I forgot to pay attention to yarn format during merge of #1408 (#1452)
eba5473
billyvg pushed a commit to getsentry/rrweb that referenced this pull request Apr 25, 2024
@avillegasn @billyvg
Expose constant SKIP_TIME_THRESHOLD as inactivePeriodThreshold in rep…
85ff9f9 
…layer (rrweb-io#1408)

Expose constant SKIP_TIME_THRESHOLD as `inactivePeriodThreshold` config setting in replayer
billyvg pushed a commit to getsentry/rrweb that referenced this pull request Apr 25, 2024
@avillegasn @billyvg
Expose constant SKIP_TIME_THRESHOLD as inactivePeriodThreshold in rep…
4b8764f 
…layer (rrweb-io#1408)

Expose constant SKIP_TIME_THRESHOLD as `inactivePeriodThreshold` config setting in replayer
@marandaneto marandaneto mentioned this pull request Apr 26, 2024
Draft
jaj1014 pushed a commit to pendo-io/rrweb that referenced this pull request Apr 30, 2024
@avillegasn @jaj1014
Expose constant SKIP_TIME_THRESHOLD as inactivePeriodThreshold in rep…
468112a 
…layer (rrweb-io#1408)

Expose constant SKIP_TIME_THRESHOLD as `inactivePeriodThreshold` config setting in replayer
jaj1014 pushed a commit to pendo-io/rrweb that referenced this pull request Apr 30, 2024
@eoghanmurray @jaj1014
I forgot to pay attention to yarn format during merge of rrweb-io#1408
95569fe 
 (rrweb-io#1452)
jxiwang pushed a commit to amplitude/rrweb that referenced this pull request Jul 31, 2024
@avillegasn @jxiwang
Expose constant SKIP_TIME_THRESHOLD as inactivePeriodThreshold in rep…
9b580ae 
…layer (rrweb-io#1408)

Expose constant SKIP_TIME_THRESHOLD as `inactivePeriodThreshold` config setting in replayer
jxiwang pushed a commit to amplitude/rrweb that referenced this pull request Jul 31, 2024
@eoghanmurray @jxiwang
I forgot to pay attention to yarn format during merge of rrweb-io#1408
3f0c4a1 
 (rrweb-io#1452)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eoghanmurray eoghanmurray eoghanmurray approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@avillegasn @eoghanmurray