Custom Info Panel position #989

mariepw · 2018-01-30T10:36:21Z

Hi @majapw, Hi @ljharb, Hi all,
I implemented the changes proposed in #840 by adding prop 'calendarInfoPosition' to 'SingleDatePicker' and 'DateRangePicker'.
I also directly updated the stories, to test thoses changes.
Thanks.

coveralls · 2018-01-30T10:41:00Z

Coverage decreased (-0.3%) to 86.513% when pulling ce7c1c0 on mariepw:master into 7dfe618 on airbnb:master.

ljharb · 2018-01-31T00:19:06Z

src/components/DayPicker.jsx

+        ref={this.setCalendarInfoRef}
+        {...css((calendarInfoIsInline) && styles.DayPicker_calendarInfo__horizontal)}
+      >
+        { renderCalendarInfo() }


nit: jsx curly braces shouldn't have padding spaces inside them

ljharb · 2018-01-31T00:23:06Z

src/components/DayPicker.jsx

+    };
+
+    const dayPickerStyle = {
+      width: this.isHorizontal() && fullHorizontalWidth,


can we cache the isHorizontal result in a var, so we don't have to call the function multiple times?

Done.. but the function is called at other different places in the code... Will it be better to cache the result in a state?

nah, that would cause a rerender. It's fine to call multiple times conceptually, but it's nice to minimize duplicate function calls within the body of a single function :-)

So, it is okay like that ?

ljharb · 2018-01-31T00:23:29Z

src/components/DayPicker.jsx

+
+      // These values are to center the datepicker (approximately) on the page
+      marginLeft: this.isHorizontal() && withPortal && -fullHorizontalWidth / 2,
+      marginTop: this.isHorizontal() && withPortal && -calendarMonthWidth / 2,


rather than overloading && for value selection, can these use explicit ternaries?

I changed it, but for the record, the implicit ternaries were already here before, I only change their position in the code..

Fair enough :-) thanks for leaving it cleaner than you found it!

My pleasure ! :)

…tiple calls | react-dates#989

majapw

Hey @mariepw, the code for this looks good and I checked out the branch as well.

Two comments:

Can we default the info position to the bottom so as not to break this for current consumers?
The height animation seems broken when the info panel is on the left or on the right:

Can you investigate a bit?

…t-dates#989

mariepw · 2018-02-01T13:43:19Z

Hi @majapw ,

1 - Totally right, sorry for the "glitch", I changed the default info position.
2 - Still under investigation, but my idea is that it is because of the change of display on wrapper and infoPanel when they are inline... And to be honest, not sure how to change this...

mariepw · 2018-02-02T12:17:11Z

I just found out where the issue is coming from... It is due to the use of calculateDimension.. Will find a way to fix this..

…" by "before | after" - react-dates#989

mariepw · 2018-02-06T13:38:36Z

I'm still stuck with this problem...

When I change :
const calendarInfoWith = renderCalendarInfo && calendarInfoIsInline ? calculateDimension(this.calendarInfo, 'width', true, true) : 0;
to :
const calendarInfoWith = renderCalendarInfo && calendarInfoIsInline ? 300 // hardcoded random value : 0;
transition works well, and calendar info panel will always have the same width.. But I cannot just hardcode a value like this...

I tried to get the width when componentDidMount and add it to a state, transition then works, but the width is not set properly... (render is called several times after componentDidMount).
Then I tried :
- to recalculate the width in componentWillUpdate, and set the state again...
- change the display of DayPicker_calendarInfo__horizontal and DayPicker_wrapper__horizontal from table-cell to inline-block
- add 1px to fullHorizontalWidth (to compensate white-space between the two inline-block)
This solution fix the problem of width for calendar info panel, but the transition is still buggy..
I even tried to use this.calendarInfo.getBoundingClientRect().width instead of calculateDimension(this.calendarInfo, 'width', true, true).. But it didn't change nothing..

So... Do you have any idea @majapw @ljharb ?
Thanks a lot!

…rtal | react-dates#989

mariepw · 2018-02-08T10:07:50Z

I finally fixed the issue..
Calculating the dimensions of calendar info panel, was creating a DOM repaint, this is what was breaking the CSS transition.. Adding a setTimeout to delay calculation, to wait until transition ends, fixed it.
Everything works now, calendar info panel renders itself with expected width, and transition works.
Let me know if it is ok.

majapw

This is awesome! Thank you so much. I have two small comments and then I think we're good to go.

Thank you so much for debugging and putting in the work @mariepw! :)

majapw · 2018-02-08T18:51:26Z

src/components/DayPicker.jsx

+            calendarInfoWidth: calendarInfoPanelWidth,
+          });
+        }
+      }, 200);


Should we use transitionDuration here instead?

Makes sense :) I didn't see the props...

majapw · 2018-02-08T18:51:55Z

src/components/DayPicker.jsx

+    // The setTimeout will wait until the transition ends.
+    if (this.calendarInfo) {
+      const { calendarInfoWidth } = this.state;
+      this.timeout = setTimeout(() => {


Can we name this this.setCalendarInfoWidthTimeout or something else meaningful

…variable name | react-dates#989

mariepw · 2018-02-09T06:02:31Z

Thanks a lot @majapw !!! Last changes are done now :)
I cannot take all the credits and have to say thanks to @onigoetz, because he gave me the lead about the DOM repaint...
Anyway, yay it's done! Thanks :)

mariepw · 2018-02-13T08:12:58Z

Hi @majapw do you know when it will be possible to merge ?

majapw · 2018-02-13T16:00:34Z

Sorry @mariepw, it slipped off my radar! :) Let me do a release today.

mariepw · 2018-02-13T16:20:29Z

Yeeaah for the release, perfect !! Thanks a lot @majapw !

Add props to make calendar info panel position configurable

71051e7

mariepw added 3 commits January 30, 2018 13:07

Fix react-dates#989 - Fix linting

62ef8c3

Fix react-dates#989 - Fix linting

f24ef36

Fix react-dates#989 - Fix linting

efcd0f5

ljharb reviewed Jan 31, 2018

View reviewed changes

mariepw added 3 commits January 31, 2018 11:22

Fix jsx curly braces | react-dates#989

ddbb26b

Fix - cache in a variable value of this.horizontal() instead of mul…

3e48478

…tiple calls | react-dates#989

Fix - Change implicit ternaries to explicit ternaries | react-dates#989

5959b04

majapw requested changes Jan 31, 2018

View reviewed changes

Fix - Set default panel info position to bottom instead of top | reac…

1ab4911

…t-dates#989

In order to be more appropriate with RTL world, replace "left | right…

ff10fff

…" by "before | after" - react-dates#989

mariepw added 2 commits February 8, 2018 11:02

Fix CSS transition problem due to width calculation | react-dates#989

ef87d78

Add another example to be sure that calendar info panel works with po…

1fd9aef

…rtal | react-dates#989

majapw reviewed Feb 8, 2018

View reviewed changes

Fix Use props value instead of a hardcoded one + Use more meaningful …

ce7c1c0

…variable name | react-dates#989

majapw approved these changes Feb 13, 2018

View reviewed changes

majapw merged commit cec101b into react-dates:master Feb 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom Info Panel position #989

Custom Info Panel position #989

mariepw commented Jan 30, 2018

coveralls commented Jan 30, 2018 •

edited

ljharb Jan 31, 2018

mariepw Jan 31, 2018

ljharb Jan 31, 2018

mariepw Jan 31, 2018

ljharb Jan 31, 2018

mariepw Jan 31, 2018

ljharb Jan 31, 2018

mariepw Jan 31, 2018

ljharb Jan 31, 2018

mariepw Jan 31, 2018

majapw left a comment

mariepw commented Feb 1, 2018 •

edited

mariepw commented Feb 2, 2018

mariepw commented Feb 6, 2018

mariepw commented Feb 8, 2018

majapw left a comment

majapw Feb 8, 2018

mariepw Feb 9, 2018

majapw Feb 8, 2018

mariepw commented Feb 9, 2018

mariepw commented Feb 13, 2018 •

edited

majapw commented Feb 13, 2018

mariepw commented Feb 13, 2018

Custom Info Panel position #989

Custom Info Panel position #989

Conversation

mariepw commented Jan 30, 2018

coveralls commented Jan 30, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

majapw left a comment

Choose a reason for hiding this comment

mariepw commented Feb 1, 2018 • edited

mariepw commented Feb 2, 2018

mariepw commented Feb 6, 2018

mariepw commented Feb 8, 2018

majapw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariepw commented Feb 9, 2018

mariepw commented Feb 13, 2018 • edited

majapw commented Feb 13, 2018

mariepw commented Feb 13, 2018

coveralls commented Jan 30, 2018 •

edited

mariepw commented Feb 1, 2018 •

edited

mariepw commented Feb 13, 2018 •

edited