skratchdot / react-bootstrap-daterangepicker

A date/time picker for react (using bootstrap). This is a react wrapper around the bootstrap-daterangepicker project.
http://projects.skratchdot.com/react-bootstrap-daterangepicker/
Other
474 stars 203 forks source link

onApply event not triggering in v5 #223

Closed vasturiano closed 4 years ago

vasturiano commented 4 years ago

Thanks for maintaining this very useful module.

After upgrading to v5.0.0, I noticed the onApply event only gets triggered the first time the user interacts with the popup window. If the user reopens the popup to select a new time range no further events are triggered.

This is behaving correctly in the previous v4.1.0.

skratchdot commented 4 years ago

@vasturiano - thanks for the report. I don't really use this lib, so have just been trying my best to maintain it (and merge prs when people submit them).

It seems like onApply is working in the storybook: https://projects.skratchdot.com/react-bootstrap-daterangepicker/?path=/story/daterangepicker--log-events

I see onApply being triggered every time I click it. I'm wondering if it's a combination of the props you are using that cause it to fail? Can you post an example of the bug on codesandbox.io so we can see it action?

skratchdot commented 4 years ago

I'm seeing a similar issue reported (before v5.0.0 was pushed): #222

@vasturiano - what version of bootstrap-daterangepicker are you using in your repo? (aka what does npm ls bootstrap-daterangepicker report)?

skratchdot commented 4 years ago

This example is working for me:

https://codesandbox.io/s/vigilant-darkness-imzb5?file=/src/App.js

Can you please change it so it stops working? Not sure what you are doing in your code, or why it's not working for you

kimze80 commented 4 years ago

Same here! Any event trigger does not work at all in v5 and using v3.1 of bootstrap-daterangepicker

` const DatePickerInput = () => { const start = moment().subtract(29, 'days'); const end = moment(); const [rangeLabel, setRangeLabel] = useState(moment().format('DD/MM/YYYY')); const handlePickDate = useCallback((e, picker) => { console.log(e); console.log(picker.chosenLabel); setRangeLabel(picker.chosenLabel); }, []);

const ranges = { Hoje: [moment(), moment()], Ontem: [moment().subtract(1, 'days'), moment().subtract(1, 'days')], 'Últimos 7 dias': [moment().subtract(6, 'days'), moment()], 'Últimos 30 dias': [moment().subtract(29, 'days'), moment()], 'Este mês': [moment().startOf('month'), moment().endOf('month')], 'Mês passado': [ moment().subtract(1, 'month').startOf('month'), moment().subtract(1, 'month').endOf('month'), ], }; const locale = { customRangeLabel: 'Customizado', daysOfWeek: ['Dom', 'Seg', 'Ter', 'Qua', 'Qui', 'Sex', 'Sab'], monthNames: [ 'Janeiro', 'Fevereiro', 'Março', 'Abril', 'Maio', 'Junho', 'Julho', 'Agosto', 'Setembro', 'Outubro', 'Novembro', 'Dezembro', ], firstDay: 1, applyLabel: 'Aplicar', cancelLabel: 'Cancelar', };

return (

); };`

skratchdot commented 4 years ago

@kimze80 - thank you! there's definitely a bug (and I can easily reproduce. please see this comment: https://github.com/skratchdot/react-bootstrap-daterangepicker/pull/216#issuecomment-663032537

when I get off work today, i'm going to make the changes. i'm not sure if i should rollback and publish 5.0.1 (to make it the same as v4) or just leave v5 broken and update the docs to not use it.

your thoughts?

kimze80 commented 4 years ago

@skratchdot My solution was to get the v4 and it is working well now. I think if I were you I will do the version 5.0.1 and make same as v4...this is my thought..

vasturiano commented 4 years ago

@skratchdot thanks for looking at this!

I would also suggest to publish a 5.0.1 version that fixes it. Or if there's no intention to bring back the other changes introduced in v5, you could bump directly to 6.0.0.

Alternatively, if it's just a plain revert, you could just move the latest tag point in NPM back to 4.1.0. https://docs.npmjs.com/cli/dist-tag#purpose https://www.npmjs.com/package/react-bootstrap-daterangepicker

skratchdot commented 4 years ago

Thank you! This should be fixed in 5.0.1 in commit 6bca87700594c62df38a372d55c64f2234c55e2e

Can you please re-test?

skratchdot commented 4 years ago

I'm going to "close" this issue, but I will re-open if this new version does not work for you. Sorry about the trouble!

vasturiano commented 4 years ago

@skratchdot just tested and confirm this is fixed in 5.0.1. Thanks!