Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix live dev highlight being out of place in edge cases #8922

Merged
merged 1 commit into from
Oct 15, 2014

Conversation

marcelgerber
Copy link
Contributor

For #7921, where Live Preview highlight is out of place when using with brackets.io site.

Tested with brackets.io/index.html, brackets.io/contribute.html, and the (German) Getting Started page.

@redmunds
Copy link
Contributor

redmunds commented Oct 7, 2014

I'm testing the root Getting Started page. It looks good when page is less than max width:

live-highlight1

But wider than max width does not offset highlight correctly:

live-highlight2

@marcelgerber
Copy link
Contributor Author

Wow, that one was tough!
I found a fix which fixes both cases.

The issue was that, as soon as <body> is not position: static, every child with position: absolute (like our highlight) is relative to the body.

@redmunds
Copy link
Contributor

This fixes the Getting started page, but the brackets.io/contribute.html page used in the recipe of #7921 is broken when scrolling. It's weird that element highlights in the header-wrapper fixed header at the top scroll when they shouldn't and element highlights in the rest of the page do not scroll when they should.

@marcelgerber
Copy link
Contributor Author

Somehow works now. Fingers crossed.

@redmunds redmunds self-assigned this Oct 14, 2014
@redmunds redmunds added this to the Brackets 1.0 (Release 0.45) milestone Oct 14, 2014
@redmunds
Copy link
Contributor

I want to consider taking this for Brackets 1.0. Need to make sure all problems are fixed and we test it enough to be assured that risk is low.

@marcelgerber
Copy link
Contributor Author

One note about testing:
We should try different position values (static, relative, absolute, fixed) for both body and the highlighted element, and various top/bottom/left/right/margin values.

@redmunds
Copy link
Contributor

@marcelgerber This looks great. Squash to a single commit and I'll merge it.

@larz0
Copy link
Member

larz0 commented Oct 15, 2014

saywhut

@marcelgerber
Copy link
Contributor Author

@redmunds Ready.

@redmunds
Copy link
Contributor

Looks good. Merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants