search

kiwicom / margarita

[not actively maintained] Mobile and Web application implementing Kiwi.com Tequila API
https://margarita.kiwi.com/
MIT License
209 stars 42 forks source link

Possibility of picking history dates in RangeDatePicker #835

Closed jvaclavik closed 5 years ago

jvaclavik commented 5 years ago

Closes #828

netlify[bot] commented 5 years ago

Storybook from packages/universal-components

Built with commit 29679fb6bf3ad378558c82cf1c75c3f9d26d9e36

https://deploy-preview-835--kiwicom-universal-components.netlify.com

jvaclavik commented 5 years ago

And two things? What is the motivation for this?

Why it should render dates only from current date without possibility of select date in history? It's nothing very important, but these two props make RangeDatePicker much more universal. Originally we found these issue when we wanted to use it for choosing birth date. It's not actual anymore, but I think it still could be useful and it was quite easy fix.

And I realize that it doesn't work properly in the storybook. When I set the ChoosingPastDatesEnabled to true, nothing happens, I can't choose before date, at first I must take a future date and then it allows me.

  1. Open the RangeDatePicker in the storybook
  2. checked the ChoosingPastDatesEnabled
  3. try to pick up past date

I also noticed that but I was thinking that I would fix this only because of Storybook. It doesn't seem to me very important, because I don't see there any use case for changing this props dynamically. I can fix it if you think it should be like that.

FilipMessa commented 5 years ago

I also noticed that but I was thinking that I would fix this only because of Storybook. It doesn't seem to me very important, because I don't see there any use case for changing this props dynamically. I can fix it if you think it should be like that.

I think that should work because of this is as the react work, if I change props I expect that component will react on that.

JosefDuda commented 5 years ago

I also noticed that but I was thinking that I would fix this only because of Storybook. It doesn't seem to me very important, because I don't see there any use case for changing this props dynamically. I can fix it if you think it should be like that.

I think that should work because of this is as the react work, if I change props I expect that component will react on that.

I'm also for fix. I understand that it is not important for real use case. But on the other hand Storybook is usually your first contact with components when you're exploring the library. And I also thought that something is broken when I played with it for a first time.

jvaclavik commented 5 years ago

I also noticed that but I was thinking that I would fix this only because of Storybook. It doesn't seem to me very important, because I don't see there any use case for changing this props dynamically. I can fix it if you think it should be like that.

I think that should work because of this is as the react work, if I change props I expect that component will react on that.

I'm also for fix. I understand that it is not important for real use case. But on the other hand Storybook is usually your first contact with components when you're exploring the library. And I also thought that something is broken when I played with it for a first time.

OK, your notes make sense. I will fix it

jvaclavik commented 5 years ago

I also noticed that but I was thinking that I would fix this only because of Storybook. It doesn't seem to me very important, because I don't see there any use case for changing this props dynamically. I can fix it if you think it should be like that.

I think that should work because of this is as the react work, if I change props I expect that component will react on that.

I'm also for fix. I understand that it is not important for real use case. But on the other hand Storybook is usually your first contact with components when you're exploring the library. And I also thought that something is broken when I played with it for a first time.

OK, your notes make sense. I will fix it

Done

FilipMessa commented 5 years ago

I am on it 🚀

FilipMessa commented 5 years ago

I realize that when I unchecked the IsRangePicker value, the behavior is to as I not expect. Will be hard to fix it?

bug