react-nice-dates icon indicating copy to clipboard operation
react-nice-dates copied to clipboard

feat(useGrid): fallback size when containerWidth is 0

Open luizcieslak opened this issue 4 years ago • 3 comments

Closes #37

I tried to write a unit test for useGrid but it fails since the returned ref must be place somewhere:

import { renderHook } from '@testing-library/react-hooks'
import { enGB } from 'date-fns/locale'
import useGrid from '../src/useGrid'

describe('useGrid', () => {
  test('should have cellHeight greated than 6px', () => {
    const { result } = renderHook(() => useGrid({ locale: enGB, onMonthChange: () => {}, transitionDuration: 300 }))

    expect(result.current.cellHeight).toBeGreaterThan(6)
  })
})

I also tried toHaveAttribute from jest-dom with no success.

It seems to solve the problem tho, I tested manually across different devices.

If you have a suggestion regarding the test, I can update the PR before merging.

luizcieslak avatar Jul 08 '20 14:07 luizcieslak

Thank you for taking the time for this!

I was just trying it locally, it works well for the default size, but if you change the container size to something else then the aspect ratio gets distorted (notice the circles on hover):

image

image

So I'm not sure if setting a fixed fallback size is actually the proper solution 🤔

hernansartorio avatar Jul 10 '20 03:07 hernansartorio

@hernansartorio hello. I started with your calendar today and it's absolutelly great with it's minimalistic and intuitive API, but unfortunatelly I faced exact same issue while trying to wrap it with popperjs' tooltips. I think the best solution here is to rewrite days to flexbox rows (instead of line-breaking technique with calc(100% / 7)) and make days having a fixed height and width, which will make calendar behavior more predictible. Trying to acomplish absolute responsivenes will guarantee you something will broke one day, so as the developer and maintainer of this package you should provide a simple static layout and let users to deal with responsiveness by themselves since they know their consumers better.

I'm sorry if this reads like I'm trying to teach you on how to do things. Just collecting my thoughts about this PR :)

UPD: just got an idea. Why don't you keep DatePicker responsive, but refactor standalone Calendar to have static layout :)

abriginets avatar Jul 16 '20 21:07 abriginets

@JamesJGoodwin hey, thanks for the input. Well, needing a responsive date picker was one of the main reasons I built it so making it fixed by default would defeat that purpose.

I still need to think about this more and play with potential solutions, but I'm beginning to think that adding a width prop could be a good alternative.

In any case, this is easily solvable on your side as I mentioned here at the end.

hernansartorio avatar Jul 17 '20 03:07 hernansartorio