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

A couple quick fixes to Popover and input styles #969

Merged
merged 4 commits into from
Jul 4, 2018

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 4, 2018

Cleaned up some popover styles

  • Reduced size of arrow
  • Removed old/unused styles
  • Fixed translate transitions:
    The transition was dependent on the supplied anchorPosition but that never got updated if the position of the popover had to be changed because of space. Now, it uses the same positioning side as the arrow to add the animation.

Before

After

Fixed background transition on inputs - Fixes #968

cchaos added 2 commits July 4, 2018 09:29
- Reducing size of arrow
- Fixing translate transitions
- Removing old/unused styles
@cchaos cchaos added the bug label Jul 4, 2018
@cchaos cchaos requested a review from chandlerprall July 4, 2018 14:17
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

code changes LGTM

@@ -184,14 +184,13 @@ export class EuiPopover extends Component {
popover: this.panel,
offset: 16,
arrowConfig: {
arrowWidth: 32,
arrowBuffer: 5,
arrowWidth: 24,
Copy link
Contributor

@chandlerprall chandlerprall Jul 4, 2018

Choose a reason for hiding this comment

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

arrowBuffer is required, even if it's set to 0. Without it the popover won't slide over.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

and slight tweak to animation timing
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM, reviewed changes and tested examples; needs changelog

@cchaos cchaos merged commit 68de58d into elastic:master Jul 4, 2018
@cchaos cchaos deleted the more-fixes branch July 4, 2018 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants