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

feat: add observer for 'selected' setter of HTMLOptionElement and try to fix issue #746 #810

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

YunFeng0817
Copy link
Member

  1. add observer for 'selected' setter of HTMLOptionElement
    If we change the 'selected' value of an option element, the value of its parent select element would be changed as well. So I add an observer for the 'selected' setter.
  2. try to fix issue <select> value is not masked when maskInputOptions is turned on #746

packages/rrweb-snapshot/src/snapshot.ts Outdated Show resolved Hide resolved
const userTriggered = event.isTrusted;
if (target && (target as Element).tagName === 'OPTION')
target = (target as Element).parentElement;
Copy link
Member

Choose a reason for hiding this comment

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

please inline the mechanism as comment at here

BTW, is there any doc/spec pointing out the parent element of option should be select?

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem, I will add it.

is there any doc/spec pointing out the parent element of option should be select
This is a good point, parent element could also be <optgroup> and <datalist> according to
image captured from OptionElement MDN.
But these two elements aren't hooked for now, so this exception could be excluded by this conditionINPUT_TAGS.indexOf((target as Element).tagName) < 0

Copy link
Member

@Yuyz0112 Yuyz0112 left a comment

Choose a reason for hiding this comment

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

LGTM

@Yuyz0112 Yuyz0112 merged commit 156b760 into master Jan 26, 2022
@YunFeng0817 YunFeng0817 deleted the option branch January 27, 2022 15:31
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