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

cool#9992 doc sign: fix missing enable on the cert chooser description entry #10135

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vmiklos
Copy link
Contributor

@vmiklos vmiklos commented Sep 26, 2024

Open the signatures dialog, add a signature, the certificate chooser
sub-dialog appears. Select a certificate, that enables e.g. the sign
button, but not the description input field for some reason.

Checking the websocket incoming traffic, the message for the working
button and the broken entry is the same:
jsdialog: { "jsontype": "dialog", "action": "action", "id": 10, "data": { "control_id": "viewcert", "action_type": "enable"}}
jsdialog: { "jsontype": "dialog", "action": "action", "id": 10, "data": { "control_id": "description", "action_type": "enable"}}
But the DOM is different:

<button class="ui-pushbutton jsdialog" id="viewcert" accesskey="V" disabled tabindex="0"><u class="access-key">V</u>iew Certificate</button>
<div class="ui-edit-container jsdialog" id="description" tabindex="0"><input class="ui-edit jsdialog" id="description-input" dir="auto" disabled></div>

Fix the problem similar to what commit
0d4e4af (jsdialog: find real input on
setText action, 2022-08-24) did for the setText action type: if there is
a child input element, work with that instead.

Probably doing so for the disable case would also make sense, but I this
dialog never disables input fields, so leave that alone for now.

Signed-off-by: Miklos Vajna [email protected]
Change-Id: I94a9a3c5fd846be86a5db175ce93ffff136881de

…n entry

Open the signatures dialog, add a signature, the certificate chooser
sub-dialog appears. Select a certificate, that enables e.g. the sign
button, but not the description input field for some reason.

Checking the websocket incoming traffic, the message for the working
button and the broken entry is the same:
jsdialog: { "jsontype": "dialog", "action": "action", "id": 10, "data": { "control_id": "viewcert", "action_type": "enable"}}
jsdialog: { "jsontype": "dialog", "action": "action", "id": 10, "data": { "control_id": "description", "action_type": "enable"}}
But the DOM is different:
<button class="ui-pushbutton jsdialog" id="viewcert" accesskey="V" disabled tabindex="0"><u class="access-key">V</u>iew Certificate</button>
<div class="ui-edit-container jsdialog" id="description" tabindex="0"><input class="ui-edit jsdialog" id="description-input" dir="auto" disabled></div>

Fix the problem similar to what commit
0d4e4af (jsdialog: find real input on
setText action, 2022-08-24) did for the setText action type: if there is
a child input element, work with that instead.

Probably doing so for the disable case would also make sense, but I this
dialog never disables input fields, so leave that alone for now.

Signed-off-by: Miklos Vajna <[email protected]>
Change-Id: I94a9a3c5fd846be86a5db175ce93ffff136881de
@vmiklos
Copy link
Contributor Author

vmiklos commented Sep 27, 2024

@caolanm could you please review this? Thanks.

This fixes the problem that the cert chooser dialog's cert list enables the sign button on cert select, but the comment of the signature is still disabled after cert select.

CC @eszkadev

@vmiklos vmiklos removed the request for review from caolanm September 27, 2024 06:43
so JSDialog.SynchronizeDisabledState will work correctly
this is followup for commit 3591fba
Fix state update of input element

Signed-off-by: Szymon Kłos <[email protected]>
Change-Id: Idba598a67454933f7192039e55cc592270a5e63d
@eszkadev
Copy link
Contributor

added update, @vmiklos please check if it fixes the issue (I was testing using different example in chart editing sidebar -> title field), if yes -> we can squash probably

@vmiklos
Copy link
Contributor Author

vmiklos commented Sep 27, 2024

@eszkadev sadly your commit breaks the use-case I have. Let me get everything in for the core side then you can see this in action locally. I'll ping you when I have that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To Review
Development

Successfully merging this pull request may close these issues.

2 participants