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

FormField: Info popover appears twice if `inline_label` is true #1084

Closed
mturley opened this issue Jan 9, 2020 · 3 comments · Fixed by #1088
Closed

FormField: Info popover appears twice if `inline_label` is true #1084

mturley opened this issue Jan 9, 2020 · 3 comments · Fixed by #1088

Comments

@mturley
Copy link
Contributor

@mturley mturley commented Jan 9, 2020

@mzazrivec , I found a bug in the FormField component that I missed when reviewing #1075. It's caused by this change: aab6fa0#diff-ad25e29f326cc9bdfc3075193546532aR105-R110

I noticed on the Configure Conversion Host wizard that if you use the default behavior of inline_label: true, the info popover renders in two different places for each field:

Screenshot 2020-01-09 17 14 21

This is only present on master and won't be an issue on ivanchuk until we backport #1075.

@mturley mturley added this to Backlog in v2v UI via automation Jan 9, 2020
@mzazrivec mzazrivec self-assigned this Jan 13, 2020
@mzazrivec

This comment has been minimized.

Copy link
Contributor

@mzazrivec mzazrivec commented Jan 13, 2020

@mturley Yep, I can see the problem as well. Though before I make the thing even worse, I better ask: what is the semantics of inline_label parameter of the FormField component?

@mturley

This comment has been minimized.

Copy link
Contributor Author

@mturley mturley commented Jan 13, 2020

@mzazrivec it looks like you added the inline_label prop in 208fd30#diff-ad25e29f326cc9bdfc3075193546532aR23 😄 I think when it's true it's supposed to use the (old) default behavior of putting the label in its own column, and when false it instead renders the label above the field in the same column.

It looks like renderInfoPopover() was part of this original label snippet that is hidden when inline_label is false, but in that commit it wasn't added to the label rendered above the field when inline_label is true. Later renderInfoPopover() was added after that line, but not behind a condition, so it's rendering twice when inline_label is true. I think we just need to put it behind that !inline_label condition.

So something like this, except maybe inside the same div as the label instead of in a fragment, not sure which would look right.

--- a/app/javascript/react/screens/App/common/forms/FormField.js
+++ b/app/javascript/react/screens/App/common/forms/FormField.js
@@ -106,8 +106,12 @@ export const FormField = ({
         </Grid.Col>
       )}
       <Grid.Col sm={Number.parseInt(controlWidth, 10) || 9} id={input.name}>
-        {!inline_label && <div style={{ fontSize: '15px', display: 'inline' }}>{label}</div>}
-        {renderInfoPopover()}
+        {!inline_label && (
+          <React.Fragment>
+            <div style={{ fontSize: '15px', display: 'inline' }}>{label}</div>
+            {renderInfoPopover()}
+          </React.Fragment>
+        )}
         {renderField()}
         {(help || (touched && error) || warning) && (
           <Form.HelpBlock>
@mzazrivec

This comment has been minimized.

Copy link
Contributor

@mzazrivec mzazrivec commented Jan 14, 2020

@mturley Well I'll be ... Yes, it was originally my code. Thanks.

Fixing PR: #1088

v2v UI automation moved this from Backlog to Done Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v2v UI
  
Done
2 participants
You can’t perform that action at this time.