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

Link to full text sources in single column view #2138

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

shinyichen
Copy link
Member

1. When in single column, the full text sources and data widget is no longer at the bottom of the page, but hidden as a drawer. Use the full text sources button to slide out.

demo adsabs harvard edu_(iPhone 6_7_8)

2. Drawer slides out from right side

demo adsabs harvard edu_(iPhone 6_7_8) (1)

3. When scrolled up, the menu becomes stick on top

Screen Shot 2021-04-06 at 1 27 34 PM

4. The navigation menu is similar

Screen Shot 2021-04-06 at 1 27 20 PM

@shinyichen shinyichen marked this pull request as ready for review April 6, 2021 20:41
@shinyichen
Copy link
Member Author

@thostetler Would you please check if I am placing the observer code in the right place?

@thostetler
Copy link
Member

@shinyichen yep abstract page manager is the place to put that.
This looks good so far to me

@csgrant00
Copy link

csgrant00 commented Apr 6, 2021 via email

Copy link
Member

@thostetler thostetler left a comment

Choose a reason for hiding this comment

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

These changes look good, I just had the one comment about deleting code vs. commenting it out.

Also, I would go ahead and squash some of these commits so you don't have things lingering in an older commit that you revert later.

#app-container {
will-change: transform;
}
// #app-container {
Copy link
Member

Choose a reason for hiding this comment

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

I would say instead of commenting out code/styles, just delete them here, we'll have a record in the blame to get it back if necessary

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 commented out code and squashed related commits

@thostetler
Copy link
Member

LGTM, I'll merge

@thostetler thostetler merged commit 9b56639 into adsabs:master Apr 8, 2021
@shinyichen shinyichen deleted the fullTextLink2 branch April 14, 2021 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants