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

check-storage-luks: Fixes for React #9355

Closed

Conversation

mareklibra
Copy link
Contributor

@mareklibra mareklibra commented Jun 11, 2018

Part of #9263 PR-split.

Element nesting structure has changed - (<table> is followed by <tbody> instead of <tr>).

Workaround for not-emitted onChange event when setting "value"
attribute of element directly.

Works fine now but would be better to introduce a generic
solution for that in a later patch.

Element nesting structure has changed (<table> is followed
by <tbody> instead of <tr>).

Workaround for not-emitted onChange event when setting "value"
attribute of <input> element directly.

Works fine now but would be better to introduce a generic
solution for that in a later patch.
@mareklibra
Copy link
Contributor Author

Addition of <tbody> is discussed in #9344 .

@mareklibra mareklibra mentioned this pull request Jun 11, 2018
44 tasks
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Looks OK to me, but I'd like @mvollmer to have a second look. Thanks!

# Workaround: React does not fire onChange event when setting input.value attribute directly
# https://github.com/facebook/react/issues/8971
# potential fix: https://codepen.io/pudgereyem/live/OWBrdv
def dialog_type_val(self, field, val):
Copy link
Member

Choose a reason for hiding this comment

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

The dialog_set_val function needs to be able to handle all types of fields, as it is used also by the high-level dialog and dialog_with_retry functions. And unfortunately, it needs to handle both React and the old Mustache dialogs at the same time... Right now only check-storage-luks runs into React dialogs, but the goal is to change all dialogs to be implemented with React, and thus dialog and dialog_with_retry should be ready to work with them.

Does using React really change the behavior of the <input> element so much? Could we maybe synthesize the onChange event?

@mvollmer
Copy link
Member

The changes related to the new tbody element are also in #9357 now. I'll see if I can et React-heavy running well enough to help with setting the value of a <input> element from the tests. I guess this is a very common thing to do, no?

@mareklibra
Copy link
Contributor Author

It's common in our case. But DOM produced by React is not meant to be updated externally, like via JQuery.

Not full list, but solution can be within:

  • mimic user's typing (recent way to deal with it)
  • ensure the onChange or similar is emitted, like [1]
  • mimic copy&paste (not sure if feasible)

Just setting value attribute will not be reliable, IMO. React can asynchronously re-render DOM so the change will be lost.

Google can find many discussions on this, namely: [2]

[1] https://codepen.io/pudgereyem/live/OWBrdv
[2] facebook/react#8971

@mvollmer
Copy link
Member

Ok, I think simulated typing is the way to go. I made #9394 for this, so I think we can close this here, right?

@mareklibra
Copy link
Contributor Author

@mvollmer , thanks. Works for me.

@mareklibra mareklibra closed this Jun 15, 2018
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.

3 participants