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

fix(subheader/sticky): Stick subheader to content instead of parent. #4276

Closed
wants to merge 1 commit into from

Conversation

topherfangio
Copy link
Contributor

The $mdSticky service used to stick it's contents to the parent of the <md-content> container. This resulted in some odd styling issues and the inability to have two lists with sticky subheaders side-by-side.

This fix appends the subheaders directly to the <md-content> to fix this issue.

Also, updated subheaders to use <div> tags instead of <h2>.

fixes #3535

@topherfangio topherfangio added the needs: review This PR is waiting on review from the team label Aug 21, 2015
@topherfangio topherfangio added this to the 0.12.0 milestone Aug 21, 2015
@topherfangio
Copy link
Contributor Author

@jelbourn @ThomasBurleson @rschmukler Can you guys please review this and let me know if you see any issues with the updates? This is related to the discussion we had in Monday's meeting (#3535), but I found a way to do it without the visual glitch I mentioned.

} else if (isScrollingDown && self.next) {
if (self.next.top - scrollTop <= 0) {
} else if (isScrollingDown) {
if (self.next && self.next.top - scrollTop <= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I know it was like this before, but could you change this?
self.next.top - scrollTop <= 0 is a less readable way of saying self.next.top <= scrollTop

The `$mdSticky` service used to stick it's contents to the parent
of the `<md-content>` container. This resulted in some odd styling
issues and the inability to have two lists with sticky subheaders
side-by-side.

This fix appends the subheaders directly to the `<md-content>` to
fix this issue.

Also, updated subheaders to use `<div>` tags instead of `<h2>`.

fixes #3535
@topherfangio
Copy link
Contributor Author

@jelbourn @ThomasBurleson As requested, I have refactored the onScroll() function and it's comments quite a bit to increase readability/understanding. In particular, I replaced the if/else if blocks with more simple if/return statements to more clearly separate the different cases and remove some duplicate code.

Can you two please take a look and provide feedback? It may be easier if you look at the whole file rather than the diff so you can see the overall structure.

@topherfangio topherfangio deleted the fix-subheader-sticky-3535 branch August 31, 2015 15:45
kennethcachia pushed a commit to kennethcachia/material that referenced this pull request Sep 23, 2015
The `$mdSticky` service used to stick it's contents to the parent
of the `<md-content>` container. This resulted in some odd styling
issues and the inability to have two lists with sticky subheaders
side-by-side.

This fix appends the subheaders directly to the `<md-content>` to
fix this issue.

Also, updated subheaders to use `<div>` tags instead of `<h2>`.

fixes angular#3535. closes angular#4276.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: review This PR is waiting on review from the team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

md-subheader / sticky discussion
2 participants