Skip to content

Conversation

@billyvg
Copy link
Member

@billyvg billyvg commented Jan 5, 2018

Had to change FormField layout a bit, only noticeable change is the padding-right which we can tweak a bit.

Reason for this is in a future PR where we have a field that overflows (API keys) and had to do various nesting of flex/non-flex boxes to support it (the error with position: absolute make things trickier)

image

@billyvg billyvg requested review from Chrissy and ckj January 5, 2018 01:20
@ghost
Copy link

ghost commented Jan 5, 2018

1 Warning
⚠️ You should update CHANGES due to the size of this PR

Generated by 🚫 danger

width: 50%;
padding-left: 10px;
position: relative;
overflow: hidden;
Copy link
Member Author

Choose a reason for hiding this comment

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

Prevent controls with a long string from overflowing


const FormFieldWrapper = styled(({highlighted, ...props}) => <Flex {...props} />)`
padding: 15px 20px;
padding: 15px 0 15px 20px;
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed right padding because of whitespace from control state icons

const FormFieldIsSaved = styled.div`
color: ${p => p.theme.green};
animation: ${fadeOut} 0.3s ease 2s 1 forwards;
position: absolute;
Copy link
Member Author

Choose a reason for hiding this comment

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

Kind of hacky, but would have weird behavior if "saved" happens followed by "error" where both will be rendered

(current prod)
image

@billyvg billyvg merged commit c875aef into master Jan 5, 2018
@billyvg billyvg deleted the feat/ui/change-form-field-layout branch January 5, 2018 18:51
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants