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

🐾 Enhance vSphere provider help text fields #854

Merged

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Jan 29, 2024

Reference: #646

Enhance and rephrase the help text fields and validations for following vSphere dialogs:

  1. Create vSphere provider - URL, VDDK, username, password.
  2. vSphere Credentials view/edit modes - username, password.
  3. vSphere details - URL and VDDK edit fields
  4. vSphere details - URL and VDDK titles icon help text

Changes include supporting formatted help text.

Note that handling the fingerprint and skip ca validation fields will be done as part of
#827.

Screenshots:

Screenshot from 2024-01-29 17-27-53
Screenshot from 2024-01-29 17-28-41
Screenshot from 2024-01-29 17-29-24
Screenshot from 2024-01-29 17-29-53
Screenshot from 2024-01-29 17-30-23
Screenshot from 2024-01-29 17-31-00
Screenshot from 2024-01-29 17-31-23
Screenshot from 2024-01-29 17-31-47
Screenshot from 2024-01-29 17-32-05

@sgratch
Copy link
Collaborator Author

sgratch commented Jan 29, 2024

cc:// @RichardHoch @anarnold97

@yaacov
Copy link
Member

yaacov commented Jan 29, 2024

@sgratch hi, the help texts should fit in one line, can you find a shorter phrasing for the vddk help texts?

Enhance and rephrase the help text fields and validations for following vSphere dialogs:

1. Create vSphere provider - URL, VDDK init image, username, password.
2. vSphere Credentials view/edit modes - username, password.
3. vSphere details - URL and VDDK edit fields
4. vSphere details - URL and VDDK titles icon help text

Changes include supporting formatted help text.

Note that handling the fingerprint and skip ca validation fields will be
done as part of
kubev2v#827.

Signed-off-by: Sharon Gratch <[email protected]>
@sgratch sgratch force-pushed the enhance-vSphere-provider-help-text-fields branch from 328dc54 to 5d6e43e Compare January 29, 2024 17:42
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
No data about Coverage
1.9% Duplication on New Code

See analysis details on SonarCloud

@sgratch
Copy link
Collaborator Author

sgratch commented Jan 29, 2024

@sgratch hi, the help texts should fit in one line, can you find a shorter phrasing for the vddk help texts?

@yaacov added a tooltip and moved the recommendation to the modal's body:
Screenshot from 2024-01-29 19-43-51
Screenshot from 2024-01-29 19-45-47

Screenshot from 2024-01-29 19-44-13

@yaacov yaacov changed the title Enhance vSphere provider help text fields 🐾 Enhance vSphere provider help text fields Jan 29, 2024
@yaacov yaacov added the enhancement Categorizes issue or PR as related to a new feature. label Jan 29, 2024
@yaacov yaacov added this to the 2.6.0 milestone Jan 29, 2024
@yaacov yaacov merged commit 4557cb8 into kubev2v:main Jan 29, 2024
7 checks passed
@sgratch sgratch deleted the enhance-vSphere-provider-help-text-fields branch January 29, 2024 18:12
@@ -11,6 +11,7 @@ import { validateFingerprint, validateNoSpaces } from '../common';
* 'default' - The default state of the form field, used when the field is empty or a value hasn't been entered yet.
* 'success' - The field's value has passed validation.
* 'error' - The field's value has failed validation.
* 'warning' - The field's value might fail the validation, but it's not mandatory and not disabling the form saving.
*/

Choose a reason for hiding this comment

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

...but the field is not mandatory and if it fails, the form saving will not be disabled."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. It's just a comment for explaining the warning validation state.
In case of a warning validation result for a field - a message will appear but the form saving will not be disabled.

Copy link
Member

Choose a reason for hiding this comment

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

The field's value might fail the validation, but it's not mandatory and not disabling the form saving.

the reason the "create" is not disabled is because the value is valid and will be used as it is.
using this phrasing, it seams like the value is not valid, but we allow it anyways, but this is not true.

a better phrasing will be:

The field's value is valid but does not fit the standard format, continue if you know what you are doing

)}
<Trans t={t} ns="plugin__forklift-console-plugin">
{
'URL of the API endpoint of the vCenter on which the source VM is mounted. Ensure that the URL includes the sdk path, usually <strong>/sdk</strong>. For example, <strong>https://vCenter-host-example.com/sdk</strong>. If a certificate for FQDN is specified, the value of this field needs to match the FQDN in the certificate.'
Copy link

@RichardHoch RichardHoch Jan 30, 2024

Choose a reason for hiding this comment

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

  1. Does sdk need formatting tags?
    2 For example: instead of For example,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's nicer to make the sdk path bold..to emphasis it's a reserved word.

<Trans t={t} ns="plugin__forklift-console-plugin">
{
"Error: The format of the provided user name is invalid. Ensure the user name doesn't include whitespace characters."
}

Choose a reason for hiding this comment

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

"does noit" is better.

@@ -11,6 +11,7 @@ import { validateFingerprint, validateNoSpaces } from '../common';
* 'default' - The default state of the form field, used when the field is empty or a value hasn't been entered yet.

Choose a reason for hiding this comment

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

"has not" is better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an internal comment, but will fix anyway.

const passwordHelperTextMsgs = {
error: t(
"Error: The format of the provided user password is invalid. Ensure the user password doesn't include whitespace characters.",
),

Choose a reason for hiding this comment

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

"does not" is better.

@RichardHoch
Copy link

@sgratch @yaacov A couple minor points, but otherwise LGTM!

sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Jan 30, 2024
…texts to Obj

Reference: kubev2v#646
Reference: kubev2v#854 (comment)
Reference: kubev2v#861 (comment)

A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes:
1. Fix vSphere and oVirt help text fields and code comments, based on doc team review.
2. Replace all reminded <Trans> text appearances from String to React Obj.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Jan 30, 2024
…texts to Obj

Reference: kubev2v#646
Reference: kubev2v#854 (comment)
Reference: kubev2v#861 (comment)

A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes:
1. Fix vSphere and oVirt help text fields and code comments, based on doc team review.
2. Replace all reminded <Trans> text appearances from String to React Obj.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Jan 31, 2024
…texts to Obj

Reference: kubev2v#646
Reference: kubev2v#854 (comment)
Reference: kubev2v#861 (comment)

A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes:
1. Fix vSphere and oVirt help text fields and code comments, based on doc team review.
2. Replace all reminded <Trans> text appearances from String to React Obj.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Jan 31, 2024
…texts to Obj

Reference: kubev2v#646
Reference: kubev2v#854 (comment)
Reference: kubev2v#861 (comment)

A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes:
1. Fix vSphere and oVirt help text fields and code comments, based on doc team review.
2. Replace all reminded <Trans> text appearances from String to React Obj.

Signed-off-by: Sharon Gratch <[email protected]>
sgratch added a commit to sgratch/forklift-console-plugin that referenced this pull request Jan 31, 2024
…texts to Obj

Reference: kubev2v#646
Reference: kubev2v#854 (comment)
Reference: kubev2v#861 (comment)

A follow up for kubev2v#854 and kubev2v#861 that includes the following fixes:
1. Fix vSphere and oVirt help text fields and code comments, based on doc team review.
2. Replace all reminded <Trans> text appearances from String to React Obj.

Signed-off-by: Sharon Gratch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants