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

Fix the Edit URL modal text for the oVirt provider #773

Merged
merged 1 commit into from
Nov 9, 2023

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Nov 7, 2023

Reference: #646

Fix the Edit URL modal text for the oVirt provider, based on the create provider help texts:

  1. Rephrase the title and help text field
  2. Support warning text message and icon

Before the fix

Screenshot from 2023-11-07 22-35-47

After the fix

Screenshot from 2023-11-07 22-36-22

Screenshot from 2023-11-07 22-36-45

Screenshot from 2023-11-07 22-37-06

Screenshot from 2023-11-07 22-38-16

@sgratch sgratch requested review from yaacov and rszwajko November 7, 2023 20:54
@sgratch sgratch added this to the 2.6.0 milestone Nov 7, 2023
@sgratch
Copy link
Collaborator Author

sgratch commented Nov 7, 2023

cc:// @RichardHoch @anarnold97

Copy link
Member

@yaacov yaacov left a comment

Choose a reason for hiding this comment

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

need rebase

@RichardHoch
Copy link

@sgratch @yaacov @anarnold97 @ahadas
After much discussion, the description for this field in the documentation was written as "URL of the API endpoint of the Red Hat Virtualization Manager (RHVM) on which the source VM is mounted. Ensure that the URL includes the path leading to the RHVM API server, usually /ovirt-engine/api. For example, https://rhv-host-example.com/ovirt-engine/api. "

Do I need to change the description in the documentation?

@yaacov
Copy link
Member

yaacov commented Nov 8, 2023

@sgratch hi, please notice #773 (comment)

@sgratch sgratch force-pushed the fix-edit-ovirt-url-help-text branch 2 times, most recently from 826a723 to c2ca59d Compare November 8, 2023 12:51
@yaacov
Copy link
Member

yaacov commented Nov 8, 2023

@RichardHoch just final check before merging :-)
the current text is OK as it is, or we need to change something ?

Fix the Edit URL modal text for the oVirt provider, based on the create provider help texts:
1. Rephrase the title and help text field
2. Support warning text message and icon

Signed-off-by: Sharon Gratch <[email protected]>
@sgratch sgratch force-pushed the fix-edit-ovirt-url-help-text branch from c2ca59d to fdb1180 Compare November 8, 2023 13:06
Copy link

sonarqubecloud bot commented Nov 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
10.5% 10.5% Duplication

@sgratch
Copy link
Collaborator Author

sgratch commented Nov 8, 2023

@RichardHoch just final check before merging :-) the current text is OK as it is, or we need to change something ?

Current text In Doc:
"URL of the API endpoint of the Red Hat Virtualization Manager (RHVM) on which the source VM is mounted. Ensure that the URL includes the path leading to the RHVM API server, usually /ovirt-engine/api. For example, https://rhv-host-example.com/ovirt-engine/api. "

Current text In UI:
"URL of the Red Hat Virtualization Manager (RHVM) API endpoint. Ensure the URL includes the "/ovirt-engine/api" path. For example: https://rhv-host-example.com/ovirt-engine/api',"

@RichardHoch
Copy link

@sgratch It took some to time to arrive at the documentation text. The UI text looks close enough to me, except it needs to end in a period, not a comma.

@yaacov yaacov merged commit e77a33b into kubev2v:main Nov 9, 2023
5 checks passed
@sgratch sgratch deleted the fix-edit-ovirt-url-help-text branch November 9, 2023 13:50
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