Merged
Conversation
jrieke
added
security-assessment-completed
change:feature
2 tasks
Contributor
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
Contributor
✅ PR preview is ready!
|
jrieke
changed the title
jrieke
changed the title
sfc-gh-bnisco
approved these changes
Jun 30, 2025
frontend/lib/src/components/widgets/DateInput/DateInput.test.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Bob Nisco <[email protected]>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+441
to
+461
|
|
||
|
|
||
| def test_quick_select_feature_visibility(app: Page): | ||
| """Test that quick select is visible for range inputs and hidden for single inputs.""" | ||
| # Test range input (index 2 is "Range, no date") | ||
| range_date_input = app.get_by_test_id("stDateInput").nth(2) | ||
| range_date_input.click() | ||
|
|
||
| # Quick select should be visible for range inputs | ||
| quick_select = app.locator('[data-baseweb="select"]') | ||
| expect(quick_select).to_be_visible() | ||
|
|
||
| # Close the calendar | ||
| app.keyboard.press("Escape") | ||
|
|
||
| # Test single date input (index 0 is "Single date") | ||
| single_date_input = app.get_by_test_id("stDateInput").first | ||
| single_date_input.click() | ||
|
|
||
| # Quick select should not be visible for single date inputs | ||
| expect(quick_select).not_to_be_visible() |
Contributor
There was a problem hiding this comment.
The test function is missing a return type annotation. Adding -> None would align with the project's typing standards for test functions and improve code clarity:
def test_quick_select_feature_visibility(app: Page) -> None:This maintains consistency with the typing conventions used throughout the test suite.
Suggested change
| def test_quick_select_feature_visibility(app: Page): | |
| """Test that quick select is visible for range inputs and hidden for single inputs.""" | |
| # Test range input (index 2 is "Range, no date") | |
| range_date_input = app.get_by_test_id("stDateInput").nth(2) | |
| range_date_input.click() | |
| # Quick select should be visible for range inputs | |
| quick_select = app.locator('[data-baseweb="select"]') | |
| expect(quick_select).to_be_visible() | |
| # Close the calendar | |
| app.keyboard.press("Escape") | |
| # Test single date input (index 0 is "Single date") | |
| single_date_input = app.get_by_test_id("stDateInput").first | |
| single_date_input.click() | |
| # Quick select should not be visible for single date inputs | |
| expect(quick_select).not_to_be_visible() | |
| def test_quick_select_feature_visibility(app: Page) -> None: | |
| """Test that quick select is visible for range inputs and hidden for single inputs.""" | |
| # Test range input (index 2 is "Range, no date") | |
| range_date_input = app.get_by_test_id("stDateInput").nth(2) | |
| range_date_input.click() | |
| # Quick select should be visible for range inputs | |
| quick_select = app.locator('[data-baseweb="select"]') | |
| expect(quick_select).to_be_visible() | |
| # Close the calendar | |
| app.keyboard.press("Escape") | |
| # Test single date input (index 0 is "Single date") | |
| single_date_input = app.get_by_test_id("stDateInput").first | |
| single_date_input.click() | |
| # Quick select should not be visible for single date inputs | |
| expect(quick_select).not_to_be_visible() | |
Spotted by Diamond (based on custom rules)
Is this helpful? React 👍 or 👎 to let us know.
Comment on lines
+450
to
+461
| quick_select = app.locator('[data-baseweb="select"]') | ||
| expect(quick_select).to_be_visible() | ||
|
|
||
| # Close the calendar | ||
| app.keyboard.press("Escape") | ||
|
|
||
| # Test single date input (index 0 is "Single date") | ||
| single_date_input = app.get_by_test_id("stDateInput").first | ||
| single_date_input.click() | ||
|
|
||
| # Quick select should not be visible for single date inputs | ||
| expect(quick_select).not_to_be_visible() |
Contributor
There was a problem hiding this comment.
The test reuses the same quick_select locator for both test cases, which could lead to flaky results. When testing the single date input, the locator should be re-queried after opening that calendar to ensure it's testing the current DOM state rather than potentially referencing elements from the previous interaction. Consider creating a new locator after clicking on the single date input to make the test more reliable.
Suggested change
| quick_select = app.locator('[data-baseweb="select"]') | |
| expect(quick_select).to_be_visible() | |
| # Close the calendar | |
| app.keyboard.press("Escape") | |
| # Test single date input (index 0 is "Single date") | |
| single_date_input = app.get_by_test_id("stDateInput").first | |
| single_date_input.click() | |
| # Quick select should not be visible for single date inputs | |
| expect(quick_select).not_to_be_visible() | |
| quick_select = app.locator('[data-baseweb="select"]') | |
| expect(quick_select).to_be_visible() | |
| # Close the calendar | |
| app.keyboard.press("Escape") | |
| # Test single date input (index 0 is "Single date") | |
| single_date_input = app.get_by_test_id("stDateInput").first | |
| single_date_input.click() | |
| # Re-query the quick select locator to ensure we're testing the current DOM state | |
| quick_select_for_single_date = app.locator('[data-baseweb="select"]') | |
| # Quick select should not be visible for single date inputs | |
| expect(quick_select_for_single_date).not_to_be_visible() |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
2 tasks
4 tasks
sfc-gh-bnisco
added a commit
that referenced
this pull request
Aug 22, 2025
#12295) ## Describe your changes - Problem: BaseWeb’s quick select returns midday-anchored Date objects (12:00), while our min/max bounds are midnight. Our validation compared full datetimes, so same-day end dates were flagged as > max. - Changes: - Normalize all selected dates to `00:00` in `handleChange` before validation and state updates. - Normalize stored values in `updateWidgetMgrState` before validation. - Tests: - Add unit test asserting a quick-select range ending "today" is accepted with `max` set to today and no error icon is shown. - Stabilize time by mocking `moment.now` and `Date`, avoiding fake timers that interfere with BaseWeb popovers. ## GitHub Issue Link (if applicable) Fixes #12293 Ref #10166, #10764 ## Testing Plan - ✅ Unit Tests (JS and/or Python) --- **Contribution License Agreement** By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
Adds BaseWeb's quick select dropdown to
st.date_inputwhen it's set up to select a date range:GitHub Issue Link (if applicable)
#11108
Testing Plan
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.