feat(okms): add a key/value editor to secret data #19830
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mathieumousnier
force-pushed
the
project/secret-manager-b2
branch
from
086168a to
ef6337b
Compare
github-actions
bot
added
the
has conflicts
mathieumousnier
force-pushed
the
project/secret-manager-b2
branch
2 times, most recently
from
75237d0 to
d8c4c92
Compare
rjamet-ovh
force-pushed
the
project/secret-manager-b2
branch
from
156e763 to
43c02a0
Compare
mathieumousnier
force-pushed
the
project/secret-manager-b2
branch
5 times, most recently
from
868619f to
91d46b8
Compare
Base automatically changed from
project/secret-manager-b2
to
release/okms-w45
October 31, 2025 07:43
mathieumousnier
force-pushed
the
feat/MANAGER-18318
branch
from
f87efc2 to
19c0aae
Compare
github-actions
bot
removed
the
has conflicts
mathieumousnier
force-pushed
the
dev/okms-01
branch
from
0c87aee to
4f071ef
Compare
github-actions
bot
added
the
has conflicts
mathieumousnier
force-pushed
the
feat/MANAGER-18318
branch
from
19c0aae to
edd09b4
Compare
github-actions
bot
removed
the
has conflicts
mathieumousnier
force-pushed
the
feat/MANAGER-18318
branch
from
edd09b4 to
94192bc
Compare
mathieumousnier
requested review from
rjamet-ovh
and removed request for
a team
github-actions
bot
added
the
has conflicts
mathieumousnier
force-pushed
the
dev/okms-01
branch
from
cec3542 to
262e230
Compare
ref: #MANAGER-18318 Signed-off-by: Mathieu Mousnier <[email protected]>
mathieumousnier
force-pushed
the
feat/MANAGER-18318
branch
from
94192bc to
2c23232
Compare
github-actions
bot
added
translation required
and removed
has conflicts
mathieumousnier
force-pushed
the
feat/MANAGER-18318
branch
from
2c23232 to
88a94cd
Compare
rjamet-ovh
reviewed
Nov 4, 2025
...ules/secret-manager/components/form/secret-data-form-field/SecretDataFormField.component.tsx
Outdated
Show resolved
Hide resolved
rjamet-ovh
reviewed
Nov 4, 2025
...ms/src/modules/secret-manager/components/secret-value-toggle/SecretValueToggle.component.tsx
Show resolved
Hide resolved
ref: #MANAGER-18318 Signed-off-by: Mathieu Mousnier <[email protected]>
ref: #MANAGER-18318 Signed-off-by: Mathieu Mousnier <[email protected]>
ref: #MANAGER-18318 Signed-off-by: Mathieu Mousnier <[email protected]>
ref: #MANAGER-18318 Signed-off-by: Mathieu Mousnier <[email protected]>
ref: #MANAGER-18318 Signed-off-by: Mathieu Mousnier <[email protected]>
mathieumousnier
force-pushed
the
feat/MANAGER-18318
branch
from
88a94cd to
52aef3e
Compare
rjamet-ovh
approved these changes
Nov 5, 2025
tibs245
reviewed
Nov 5, 2025
| @@ -0,0 +1,195 @@ | |||
| import { | |||
There was a problem hiding this comment.
Maybe you can use it.each if you want simplify
There was a problem hiding this comment.
yeah right, but I'm not a big fan to be honnest
tibs245
reviewed
Nov 5, 2025
...s/okms/src/modules/secret-manager/components/form/key-values-editor/KeyValuesEditor.spec.tsx
Show resolved
Hide resolved
tibs245
reviewed
Nov 5, 2025
Comment on lines
+79
to
+91
| <OdsButton | ||
| data-testid={KEY_VALUES_EDITOR_TEST_IDS.pairItemDeleteButton(index)} | ||
| className={clsx('mt-5', { | ||
| 'opacity-0 group-hover:opacity-100 transition-opacity duration-100': !isDeletable, | ||
| 'opacity-100': isDeletable, | ||
| })} | ||
| size="sm" | ||
| variant="ghost" | ||
| icon="trash" | ||
| label="" | ||
| onClick={onDelete} | ||
| isDisabled={!isDeletable} | ||
| /> |
There was a problem hiding this comment.
Maybe you can add a
aria-label={t(`${NAMESPACES.ACTIONS}:delete`)}
for accessibility
tibs245
reviewed
Nov 5, 2025
Comment on lines
+27
to
+29
| <div> | ||
| <SecretValueToggle state={toggleState} onChange={setToggleState} /> | ||
| </div> |
There was a problem hiding this comment.
Why div ? Maybe a class block can resolve your display problem ?
There was a problem hiding this comment.
I replaced "flex flex-col gap-6" by "space-y-6" and I don't need the div anymore.
This gives me a good reason to use "space-y-N" for spacing instead of "flex flex-col gap-6", I didn't have one to sell it to @rjamet-ovh ! :)
Context for Romain if you read this: flex has side-effect on its content, that "space-y-6" does not have:
tibs245
approved these changes
Nov 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket Reference: #MANAGER-18318