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

Update DateMonth #303

Merged
merged 6 commits into from
Sep 23, 2017
Merged

Update DateMonth #303

merged 6 commits into from
Sep 23, 2017

Conversation

gthomas-appfolio
Copy link
Contributor

@gthomas-appfolio gthomas-appfolio commented Sep 22, 2017

WIP, will update with more info on changes

- Move DateMonth -> MonthInput using same code as DateInput
- Add tests
@gthomas-appfolio
Copy link
Contributor Author

gthomas-appfolio commented Sep 22, 2017

This PR removes the DateMonth component and replaces with MonthInput, which uses the same interface and behavior as DateInput:

  • Removes most custom CSS
  • Removes DIY onClick outside stuff
  • The inner picker is a new MonthCalendar component that uses i18n formats for month names
  • Added unit tests
  • Inherits the DateInput behavior for onBlur formatting, date visibility, arrow keys, etc.

screen shot 2017-09-22 at 11 20 13 am


import styles from './MonthCalendar.scss';

function range(start, end) {
Copy link

Choose a reason for hiding this comment

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

We could share this in a util folder potentially

const { format } = Fecha;

const Label = ({ selected, label, onClick, visible = true }) => (
<li className={`px-0 py-1 text-center ${selected ? 'bg-primary text-white' : ''} ${!visible ? 'invisible' : ''}`} onClick={onClick}>
Copy link

Choose a reason for hiding this comment

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

  • elements should not be used for clickable surfaces. Instead we should use a `` inside it. This wil make sure it's accessible (including keyboard nav)
  • .picker {
    min-width: 16em;
    li {
    cursor: pointer;
    Copy link

    Choose a reason for hiding this comment

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

    can be removed once we're using buttons.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    Cool thanks, will check this out

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    I will mark as TODO for the release, after looking at it am porting markup used in app that had various tweaks to ensure we can click without dead areas, overlap, etc., and started re-introducing issues when I added links.

    We should/will update this, will also create issue.

    @gthomas-appfolio gthomas-appfolio merged commit b58f00a into master Sep 23, 2017
    @gthomas-appfolio gthomas-appfolio deleted the updateDateMonth branch September 23, 2017 01:38
    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.

    2 participants