fix: Apply filterOption to custom select even with options list

[rapsealk]

Hello react-js-cron!

First of all, thanks for maintaining the project.

I was looking for a way to limit week-days; for example, I want to disallow users to select Saturday and Sunday from week-days custom selector. I found that filterOption has been added since v4.1.0 and tried to use it. It works fine with other types except for week-days and months, which pass additional argument optionsList to <CustomSelect />. And I finally figured out that .filter(filterOption) is not called when optionsList exists.

🤔 This is a ...

• New feature
• Bug fix
• Demo update
• Component style update
• TypeScript definition update
• Bundle size optimization
• Performance optimization
• Refactoring
• Code style optimization
• Test Case
• README update
• Other (about what?)

🔗 Related issue link

I haven't reported an issue.

💡 Background and solution

Try this with and without the PR:

<Cron
  ...
  dropdownsConfig={{
    "week-days": {
      filterOption: ({ value }) => Number(value) < 3,
    },
  }}
/>

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

• Demo in storybook is updated/provided or not needed
• TypeScript definition is updated/provided or not needed
• Tests are updated and passed without a decrease in coverage
• Format (lint & prettier) script passed
• Build script is working
• README API section is updated or not needed

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c3e021a) 87.00% compared to head (a785652) 87.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master      #64   +/-   ##
=======================================
  Coverage   87.00%   87.00%           
=======================================
  Files          12       12           
  Lines         531      531           
  Branches      234      234           
=======================================
  Hits          462      462           
  Misses         11       11           
  Partials       58       58           

Files	Coverage Δ
src/components/CustomSelect.tsx	73.25% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[xrutayisire]

Hi,

Nice catch! Thank you very much for this NICE fix! 🙏
Can you fix the small prettier issue so I can merge?

Also, if you want to quickly add a test for what you did it's perfect, if not I could do it in the future 😉

[rapsealk]

@xrutayisire Thanks for the comment! I've just applied prettier and pushed.
I'm afraid that I'm almost new to writing test code for frontend, but I will try based on Cron.updateValue.test.tsx.
See you in an hour.

[xrutayisire]

Exactly you should have some guidance, if you don't succeed, no problem, I will add some after and you could check them.
Thank you!

[rapsealk]

@xrutayisire I was trying to check that there is only from "Thursday""Wednesday" to "Saturday" but no "Monday", but I failed.
Would you mind if I skip for this time and add test code on next PR?

The code is below:

import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import Cron from '../Cron';
import { CronType, DropdownsConfig, PeriodType } from '../types';

const WEEK_DAY_WEDNESDAY = 3
const MONTH_OCTOBER = 9

describe('Cron filter week-days', () => {
  it('should ', async () => {
    const user = userEvent.setup()
    const value = '* * * * *'
    const setValue = jest.fn()

    const allowedPeriods: PeriodType[] = ['minute', 'month', 'year']
    const allowedDropdowns: CronType[] = ['period', 'week-days']

    const dropdownsConfig: DropdownsConfig = {
      "week-days": {
        filterOption: ({ value }) => Number(value) > WEEK_DAY_WEDNESDAY,
      },
      // "months": {
      //   filterOption: ({ value }) => Number(value) > MONTH_OCTOBER,
      // },
    }

    render(
      <Cron
        value={value}
        setValue={setValue}
        // allowedPeriods={allowedPeriods}
        allowedDropdowns={allowedDropdowns}
        dropdownsConfig={dropdownsConfig}
      />
    )

    // Open Period dropdown
    await waitFor(() => {
      user.click(screen.getByText('minute'))
    })

    // Select year period
    await waitFor(() => {
      user.click(screen.getByText('year'))
    })

    // await waitFor(() => {
    //   expect(screen.getByTestId('select-period').textContent).toContain('year')
    // })

    // Open week days dropdown
    await waitFor(() => {
      user.click(screen.getByTestId('custom-select-week-days'))
      // user.click(screen.getByText('every day of the week'))
    })

    // Check dropdowns value
    await waitFor(() => {
      // expect(screen.findByText('Monday')).not.toBeInTheDocument()
      // expect(screen.findByText('FRI')).toBeNull()
      user.click(screen.getByText('Friday'))
    })
  })
})

[xrutayisire]

No problem, I will try to add some tests in master soon. I will release tonight your fix.
Thank you again!

[rapsealk]

@xrutayisire May I ask a question? It seems that there is no GHA workflow related to deployment in this repository. Are you deploying new releases manually or in other way?

[xrutayisire]

Sorry, I couldn't find the time. I will do it now and yes it's currently manually.
Here is the test I did: 95af1fa

[xrutayisire]

You can use v5.0.1

[rapsealk]

@xrutayisire Sorry, I was not meaning to rush you, but thanks for the quick release!
Do you have any plan to add a GHA workflow for automated release? There are many useful actions such as marvinpinto/action-automatic-releases. If you don't mind, I will make a new PR for this when I have time.
Thanks again!

[xrutayisire]

Not really planned. I did some setup like that in the past. This release process is pretty simple so I never did it, but yeah why not in the future 🙂 No sure about the necessary to use an external use like that tought. Release is so simple for this lib that it may be overkill.

[xrutayisire]

Thanks for everything and happy for new PRs 🙂