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

Hide invisible first month using visibility: hidden #940

Merged
merged 1 commit into from
Jan 17, 2018

Conversation

majapw
Copy link
Collaborator

@majapw majapw commented Jan 3, 2018

@majapw majapw added the semver-patch: fixes/refactors/etc Anything that's not major or minor. label Jan 3, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.257% when pulling 0721387 on maja-hide-invisible-months-for-vo into 1331539 on master.

Copy link
Contributor

@backwardok backwardok left a comment

Choose a reason for hiding this comment

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

does the animation still work as expected? Or do you need to add a visibility to the transition list (i'm not sure where that styling is set)?

@majapw
Copy link
Collaborator Author

majapw commented Jan 3, 2018

@backwardok yeah so this is in the CalendarMonthGrid_month__hideForAnimation styles which means it def gets removed when animating. I guess i should probably hide the last month too since this only applies to the first month.

@ljharb ljharb added the accessibility Anything related to ensuring *everybody* can use react-dates <3 label Jan 3, 2018
@majapw majapw force-pushed the maja-hide-invisible-months-for-vo branch from 0721387 to 6ccee38 Compare January 16, 2018 18:24
@majapw
Copy link
Collaborator Author

majapw commented Jan 16, 2018

@backwardok @erin-doyle @ljharb can you take another look?

The effect here is that both the first and last month (which are off-screen) are hidden from the nav. However, both have visibility during animation (otherwise things get weird). Thoughts?

@backwardok
Copy link
Contributor

Reasonable to me - I'll let someone else stamp

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

seems legit.

any possibility of a regression test? no worries if not.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.275% when pulling 6ccee38 on maja-hide-invisible-months-for-vo into 44f3842 on master.

@erin-doyle erin-doyle self-requested a review January 17, 2018 12:48
Copy link
Collaborator

@erin-doyle erin-doyle left a comment

Choose a reason for hiding this comment

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

👍

@majapw majapw merged commit 4e49224 into master Jan 17, 2018
@majapw majapw deleted the maja-hide-invisible-months-for-vo branch January 17, 2018 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Anything related to ensuring *everybody* can use react-dates <3 semver-patch: fixes/refactors/etc Anything that's not major or minor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants