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 input-popover bug in docs #241

Closed
wants to merge 4 commits into from
Closed

Conversation

cmslewis
Copy link
Contributor

@cmslewis cmslewis commented Nov 25, 2016

PR checklist

What changes did you make?

In the Blueprint docs for <Popover>s:

  • Disabled autofocus in the "Input" example to fix a weird scrolling bug

expected

Is there anything you'd like reviewers to focus on?

onChange={this.handleSliderChange}
value={this.state.sliderValue}
/>,
<div style={{ width: "300px" }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

this is awkward; should .pt-slider just get a min-width instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes i think that's the better solution. other inputs set their own widths so slider can follow suit. it'll make everyone's lives easier. there have been other issues reported with slider sizing (i think internally)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went ahead and did this in a separate, self-contained PR: #266

@llorca llorca removed the P3 label Nov 29, 2016
@cmslewis cmslewis changed the title Fix popover bugs in docs Fix input-popover bug in docs Nov 29, 2016
@cmslewis
Copy link
Contributor Author

cmslewis commented Nov 29, 2016

@adidahiya @giladgray - changed the scope of this PR to just fix the input-popover issue; the slider-popover issue is fixed in the self-contained #266, since it went beyond a simple docs fix.

@blueprint-bot
Copy link

Fix whitespace error

Preview: docs | table Coverage: core | datetime

@blueprint-bot
Copy link

Remove hardcoded-width parent for slider popover example

Preview: docs | table Coverage: core | datetime

@llorca
Copy link
Contributor

llorca commented Nov 29, 2016

@cmslewis I feel like the autofocus on the input makes for a good example. Isn't the scrolling bug related to PR #187?

@cmslewis
Copy link
Contributor Author

cmslewis commented Nov 29, 2016

@llorca Should we merge this as a quick fix for the rank brokenness of that example, then revisit? Alternative is to leave it in place as a reminder, but I feel like the issue/PR here in Github can track that for us.

EDIT: I checked out the preview for #187. That does fix the issue with the input popover.

@giladgray
Copy link
Contributor

i think position: fixed on overlays should resolve this issue, but we won't know till we try!

@cmslewis cmslewis closed this Nov 29, 2016
@cmslewis
Copy link
Contributor Author

Closed in favor of a better fix with position: fixed.

@adidahiya adidahiya deleted the cl/docs-popover-bugs branch December 4, 2016 16:53
@cmslewis cmslewis restored the cl/docs-popover-bugs branch December 6, 2016 00:10
@adidahiya adidahiya deleted the cl/docs-popover-bugs branch December 23, 2016 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants