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

Fix clearable #599

Merged
merged 11 commits into from
Aug 8, 2019
Merged

Fix clearable #599

merged 11 commits into from
Aug 8, 2019

Conversation

byronz
Copy link
Contributor

@byronz byronz commented Aug 7, 2019

this PR fixes both #593 and #594

@@ -53,6 +54,11 @@ jobs:
python --version
npm run test

- run:
name: Percy TearDown
Copy link
Contributor Author

@byronz byronz Aug 7, 2019

Choose a reason for hiding this comment

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

there are numerous requests on circleci community about conditional Jobs upon previous failure. right now, the behavior is when the previous job fails, the percy finalize job cannot be executed.

a compromise for now as a percy clean up when failed

DAY_SELECTOR = 'div[data-visible="true"] td.CalendarDay'


@pytest.mark.DCC594
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is an idea I have in mind to associate the test case with issue number, could be helpful for test selection by associating the test case with the issue

Copy link
Collaborator

Choose a reason for hiding this comment

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

@byronz we can keep this if it's useful, but we should do something to clear the warning it currently gives:

/Library/Frameworks/Python.framework/Versions/3.7/lib/python3.7/site-packages/_pytest/mark/structures.py:324: PytestUnknownMarkWarning:
Unknown pytest.mark.DCC594 - is this a typo? You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/latest/mark.html

oldMomentDates.end_date
) {
setProps({start_date: null, end_date: null});
}
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet Aug 8, 2019

Choose a reason for hiding this comment

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

While the previous code was not optimal in terms of setProps, it would have updated the values in all cases. Now this won't update if one is unset, whether updatemode is single or both. Rather than preventing the update if one of start/end was not previously set, could instead check and add each individually if the "global" condition is met.

e.g.

if (clearable && (!start_date || !end_date)) {
    const props = { };
    if (!start_date && oldMomentDates.start_date) {
        props.start_date = null;
    }
    // idem for end_date

    setProps(props);
}

!start_date &&
!end_date &&
(oldMomentDates.start_date !== start_date ||
oldMomentDates.end_date !== end_date)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Marc-Andre-Rivet when you click the x, both start and end will be set to null, the condition can be either one date changed .

@byronz byronz merged commit 9f1a439 into master Aug 8, 2019
@byronz byronz deleted the fix-clearable branch August 8, 2019 20:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants