Order filtering by date #2860

joykare · 2017-09-14T18:01:07Z

Resolves #2359

How to test

Make a couple of orders (better if you change the time orders are created to observe that the date filter works)
Pick a range of dates
Observe that orders are filtered correctly
Try filtering by status and searching for a particular group of orders
Observe that the date filter works in tandem with the other filter and search

rymorgan · 2017-09-15T00:40:11Z

@joykare here are my thoughts on the date picker styling. https://zpl.io/bz8M3ME

joykare · 2017-09-19T16:59:38Z

@rymorgan I did not add question-label at the bottom of the calendar because it does not have any functionality (the keyboard shortcuts on react-dates don't work with this particular implementation).

Otherwise good for a quick check

….com/reactioncommerce/reaction into joykare-order-filtering-by-date-2359

rymorgan · 2017-09-19T17:27:55Z

@joykare I'll check it out. I only added the question mark b/c I thought the react calendar had that to explain how to use the calendar. But, I think, not including it is fine. I'll check out your PR.

rymorgan · 2017-09-19T19:15:43Z

Some small design tweaks. Looking pretty good though!

rymorgan · 2017-09-19T19:28:39Z

Also, the date picker doesn't seem to work for end-date. In the following video I can see my order for the 18th when it's my start-date but not for end-date.

rymorgan · 2017-09-19T20:22:16Z

Also, one more thing for the rollover state it looks can you use the hover color from here:
https://styleguide.reactioncommerce.com/reaction/guide/colors

This one:
@rui-info-hover

brent-hoover

Haven't tested yet but just some minor style questions

brent-hoover · 2017-09-19T21:53:15Z

imports/plugins/core/orders/client/components/orderActions.js


 class OrderActions extends Component {
  static propTypes = {
-    className: PropTypes.string,
+    className: PropTypes.object,


This variable name sounds like it should be a string but it appears it's not now. Can we give it a more descriptive name?

brent-hoover · 2017-09-19T21:55:02Z

imports/plugins/core/orders/client/components/orderDashboard.js

@@ -18,6 +18,7 @@ class OrderDashboard extends Component {
    handleSelect: PropTypes.func,
    isLoading: PropTypes.object,
    multipleSelect: PropTypes.bool,
+    onDatesChange: PropTypes.func,


Is there some sort of pattern you are using for naming functions onThisChange as opposed to handleThisChange? If not I would like to standardize on one or the other. If so, let's codify this in a style guide somewhere so we can be consistent.

brent-hoover · 2017-09-19T21:56:41Z

imports/plugins/core/ui/client/components/calendarPicker/calendarPicker.js

+import React, { Component } from "react";
+import PropTypes from "prop-types";
+import { registerComponent } from "@reactioncommerce/reaction-components";
+import { DayPickerRangeController } from "react-dates";


import order

brent-hoover · 2017-09-19T22:00:03Z

imports/plugins/core/ui/client/components/calendarPicker/calendarPicker.js

+    });
+
+    if (this.props.onDatesChange) {
+      this.props.onDatesChange(startDate, endDate);


Why are we wrapping this in a conditional? Will this function not always exist? And if not shouldn't we throw an error? I feel like this is an anti-pattern that has crept into our code somehow.

Also, maybe this is unavoidable but it seems like having this function and the one passed in via props both having the same name is a recipe for a bug down the road?

The reason we have that conditional is b/c you dont have to have a prop function for when dates change; but in case you want to add some operations/ or change some stuff onDateChange specific to the component that extends the calendarPicker you could do that under this function.
Other example: https://github.com/reactioncommerce/reaction/blob/master/imports/plugins/core/ui/client/components/menu/menu.js#L7

Ok, makes sense to me

brent-hoover · 2017-09-19T22:01:33Z

imports/plugins/included/default-theme/client/index.js

@@ -3,3 +3,4 @@ import "./favicons";

 // Scripts
 import "bootstrap/dist/js/npm.js";
+


Let's not add a blank line for no reason

rymorgan · 2017-09-19T22:07:53Z

Also, it's not working responsively.

I think we should -
2. HIde shipping status on the phone (and maybe completely if it's not going to make it into marketplace)
3. Make the date picker dropdown fit on mobile. I'm cool with the arrow/triangle being centered?
4. Move the search text and icon to left align with orders.

brent-hoover · 2017-09-20T02:17:26Z

Ran into one problem with filter but doesn't seem to be related to this PR. (#2902). Otherwise this seems to work well. Just had some minor style things to address and this should be ready to go.

….com/reactioncommerce/reaction into joykare-order-filtering-by-date-2359

joykare · 2017-09-20T20:40:31Z

@zenweasel @kieha @rymorgan good for another look

rymorgan · 2017-09-20T21:42:17Z

Now, when I select a date range the first date isn't showing up. So in the case, I have a bunch of orders on the 18th that don't know up, even though it's selected as the start date for my date range.

rymorgan · 2017-09-20T21:53:18Z

In anticipation of merging this PR I made a bug for the mobile responsive issue #2910

brent-hoover · 2017-09-20T22:34:17Z

@kieha We're still waiting for your official blessing on this

joykare · 2017-09-20T22:37:30Z

@rymorgan fixed the select. Works both ways now.

kieha

Works well. LGTM. 👍🏽

brent-hoover · 2017-09-20T22:54:00Z

@joykare Is making the filter disappear at more narrow width a quick fix you can do here rather than having it linger in another ticket?

joykare · 2017-09-20T23:00:53Z

@zenweasel not as quick a fix as I thought initially, so I asked Ryan to have that as a separate ticket so that we can move along with getting all the filters in.

brent-hoover · 2017-09-20T23:18:23Z

Because BH is not working running ESLint gives me these errors

/Users/brenthoover/Projects/js/reaction/imports/plugins/core/ui/client/components/calendarPicker/calendarPicker.js
  40:1  error  Expected indentation of 4 spaces but found 2  indent
  42:1  error  Expected indentation of 4 spaces but found 2  indent
  43:1  error  Expected indentation of 6 spaces but found 4  indent
  44:1  error  Expected indentation of 6 spaces but found 4  indent
  45:1  error  Expected indentation of 6 spaces but found 4  indent
  46:1  error  Expected indentation of 6 spaces but found 4  indent
  47:1  error  Expected indentation of 4 spaces but found 2  indent
  49:1  error  Expected indentation of 4 spaces but found 2  indent
  50:1  error  Expected indentation of 6 spaces but found 4  indent
  51:1  error  Expected indentation of 8 spaces but found 6  indent
  52:1  error  Expected indentation of 8 spaces but found 6  indent
  53:1  error  Expected indentation of 8 spaces but found 6  indent
  54:1  error  Expected indentation of 8 spaces but found 6  indent
  55:1  error  Expected indentation of 8 spaces but found 6  indent
  56:1  error  Expected indentation of 8 spaces but found 6  indent
  57:1  error  Expected indentation of 8 spaces but found 6  indent
  58:1  error  Expected indentation of 8 spaces but found 6  indent
  59:1  error  Expected indentation of 8 spaces but found 6  indent
  60:1  error  Expected indentation of 6 spaces but found 4  indent
  61:1  error  Expected indentation of 4 spaces but found 2  indent
  62:1  error  Expected indentation of 2 spaces but found 0  indent

joykare added 8 commits September 13, 2017 15:34

Install and create dummy component for dates

f07ed43

Styling around component

168bf0d

Styling around calendar

7b09563

Redo calendar picker

7e37e8d

Revamp of the calendar picker more props

c0a8265

Remove unnecessary packages

c13ae4b

CHange RTL to false

b730765

Change Date range label

304c0e8

joykare added 8 commits September 15, 2017 14:56

Merge branch 'marketplace' into joykare-order-filtering-by-date-2359

5104f60

Inner table fixes

39226b2

Restructure less file

bc4c5f0

Fix on click error

55bc7d0

Clear filters

6eda5e4

Show range on filter

6ba8cf7

package.json

654bd9f

Merge branch 'marketplace' into joykare-order-filtering-by-date-2359

85945dd

joykare changed the title ~~[WIP] Order filtering by date~~ Order filtering by date Sep 19, 2017

Merge branch 'marketplace' into joykare-order-filtering-by-date-2359

262bdd5

joykare requested review from brent-hoover and kieha September 19, 2017 16:59

joykare added 2 commits September 19, 2017 20:19

Revert back versions of react and react-dom

d6b8780

Merge branch 'joykare-order-filtering-by-date-2359' of https://github…

bcb596f

….com/reactioncommerce/reaction into joykare-order-filtering-by-date-2359

brent-hoover requested changes Sep 19, 2017

View reviewed changes

Merge branch 'marketplace' into joykare-order-filtering-by-date-2359

f135ef5

joykare added 6 commits September 20, 2017 15:50

Style enhancements

7000c40

Add inclusion of actual date

408abfe

Change date query

9aebaae

Merge branch 'joykare-order-filtering-by-date-2359' of https://github…

6485665

….com/reactioncommerce/reaction into joykare-order-filtering-by-date-2359

Code clean up

7262a4f

Have end date included in search

ee0c07b

brent-hoover approved these changes Sep 20, 2017

View reviewed changes

Date pick fix to include selected days

a14d0ad

Merge branch 'marketplace' into joykare-order-filtering-by-date-2359

235e80c

kieha reviewed Sep 20, 2017

View reviewed changes

kieha approved these changes Sep 20, 2017

View reviewed changes

Merge branch 'marketplace' into joykare-order-filtering-by-date-2359

477e976

Indentation fix

4ebd37a

brent-hoover merged commit e4a8cb1 into marketplace Sep 20, 2017

brent-hoover deleted the joykare-order-filtering-by-date-2359 branch September 20, 2017 23:24

brent-hoover mentioned this pull request Sep 20, 2017

Filter orders by created at date #2359

Closed

spencern mentioned this pull request Oct 11, 2017

Release 1.5.0 #3089

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Order filtering by date #2860

Order filtering by date #2860

joykare commented Sep 14, 2017 •

edited

Loading

rymorgan commented Sep 15, 2017

joykare commented Sep 19, 2017

rymorgan commented Sep 19, 2017

rymorgan commented Sep 19, 2017

rymorgan commented Sep 19, 2017 •

edited

Loading

rymorgan commented Sep 19, 2017

brent-hoover left a comment

brent-hoover Sep 19, 2017

brent-hoover Sep 19, 2017

brent-hoover Sep 19, 2017

brent-hoover Sep 19, 2017

joykare Sep 20, 2017

brent-hoover Sep 20, 2017

brent-hoover Sep 19, 2017

rymorgan commented Sep 19, 2017 •

edited

Loading

brent-hoover commented Sep 20, 2017

joykare commented Sep 20, 2017 •

edited

Loading

rymorgan commented Sep 20, 2017 •

edited

Loading

rymorgan commented Sep 20, 2017

brent-hoover commented Sep 20, 2017

joykare commented Sep 20, 2017

kieha left a comment

brent-hoover commented Sep 20, 2017

joykare commented Sep 20, 2017

brent-hoover commented Sep 20, 2017

		@@ -3,3 +3,4 @@ import "./favicons";

		// Scripts
		import "bootstrap/dist/js/npm.js";

Order filtering by date #2860

Order filtering by date #2860

Conversation

joykare commented Sep 14, 2017 • edited Loading

How to test

rymorgan commented Sep 15, 2017

joykare commented Sep 19, 2017

rymorgan commented Sep 19, 2017

rymorgan commented Sep 19, 2017

rymorgan commented Sep 19, 2017 • edited Loading

rymorgan commented Sep 19, 2017

brent-hoover left a comment

Choose a reason for hiding this comment

brent-hoover Sep 19, 2017

Choose a reason for hiding this comment

brent-hoover Sep 19, 2017

Choose a reason for hiding this comment

brent-hoover Sep 19, 2017

Choose a reason for hiding this comment

brent-hoover Sep 19, 2017

Choose a reason for hiding this comment

joykare Sep 20, 2017

Choose a reason for hiding this comment

brent-hoover Sep 20, 2017

Choose a reason for hiding this comment

brent-hoover Sep 19, 2017

Choose a reason for hiding this comment

rymorgan commented Sep 19, 2017 • edited Loading

brent-hoover commented Sep 20, 2017

joykare commented Sep 20, 2017 • edited Loading

rymorgan commented Sep 20, 2017 • edited Loading

rymorgan commented Sep 20, 2017

brent-hoover commented Sep 20, 2017

joykare commented Sep 20, 2017

kieha left a comment

Choose a reason for hiding this comment

brent-hoover commented Sep 20, 2017

joykare commented Sep 20, 2017

brent-hoover commented Sep 20, 2017

joykare commented Sep 14, 2017 •

edited

Loading

rymorgan commented Sep 19, 2017 •

edited

Loading

rymorgan commented Sep 19, 2017 •

edited

Loading

joykare commented Sep 20, 2017 •

edited

Loading

rymorgan commented Sep 20, 2017 •

edited

Loading