Create `meal-order-menu-container` component. by PaulAgeyev · Pull Request #53 · Java-Classes/MealOrder

PaulAgeyev · 2018-03-29T16:14:39Z

In this PR:

Created the following routing:

/#/menu/2018-03-21
/#/history/2018-02-22

Created meal-order-menu-container component that contains paper-tabs component that contains days for ordering dishes.
Created meal-order-drawer in the meal-order-client-app component that contains
app-datepicker.

View on desktop and on mobile device:

…omponent

… component

dkalinin

@PaulAgeyev

dkalinin · 2018-04-26T17:52:48Z

@@ -19,9 +19,8 @@
 -->
 <link rel="import" href="shell/meal-order-app-namespace.html">


URLs should be relative. First is block with dependencies imports, then after empty line goes block with project imports.

dkalinin · 2018-04-26T17:54:14Z

@@ -19,9 +19,8 @@
 -->
 <link rel="import" href="shell/meal-order-app-namespace.html">
 <script src="../../bower_components/webcomponentsjs/webcomponents-lite.js"></script>


Please remove

dkalinin · 2018-04-26T17:56:26Z

+ icon="icons:date-range"
+ drawer-toggle></iron-icon>
+ </div>
+ </iron-pages>


Please extract the <...toolbar> component.

dkalinin · 2018-04-26T17:57:27Z

+ *
+ * @typedef {Object} Menu the menu for the current date
+ *
+ * @property {String} Vendor a vendor name


Should be vendor

dkalinin · 2018-04-26T17:58:43Z

+ ];
+ }
+
+ _getClassForDrawer(page) {


_getDrawerClass

dkalinin · 2018-04-26T18:00:22Z

+ route="{{route}}"
+ pattern="/:page/:date"
+ data="{{routeData}}">
+ </app-route>


This is done in <app> component.

Can you please describe in more detail?

Is it good practice to change routing by two way binding in this case?

dkalinin · 2018-04-26T18:01:18Z

+ _changeSelectedDay() {
+ const date = new Date(this.routeData.date);
+
+ let dayOfTheWeek = date.toLocaleDateString("ru-RU", {weekday: 'long'});


Please use const and method chain.

dkalinin · 2018-04-26T18:02:00Z

+
+ const day = {
+ day: date.getDate(),
+ dayOfTheWeek: dayOfTheWeek,


Why not to create two properties?

dkalinin · 2018-04-26T18:02:45Z

+ */
+ selectedDay: {
+ type: Object,
+ computed: '_changeSelectedDay(routeData.date)'


_changeSelectedDay -> getSelectedDay

dkalinin · 2018-04-26T18:02:52Z

+ */
+ inputDate: {
+ type: String,
+ computed: '_changeCalendarDate(routeData.date)',


_changeSelectedDay -> getSelectedDay

dkalinin

@PaulAgeyev PTAL

dkalinin · 2018-04-27T08:22:39Z

+ }
+ }
+
+ _drawerToggle() {


_toggleDrawer

dkalinin · 2018-04-27T08:24:11Z

+ }
+ if (this.routeData.page.startsWith("history")) {
+ if (!this.routeData.date) {
+ let yesterday = new Date(this.serverTime);


Is server time string or Object?

dkalinin · 2018-04-27T08:27:52Z

+ }
+ if (this.routeData.page.startsWith("history")) {
+ if (!this.routeData.date) {
+ let yesterday = new Date(this.serverTime);


yesterday is not yesterday, it's a server date.

dkalinin · 2018-04-27T08:28:16Z

+ }
+ if (this.routeData.page.startsWith("history")) {
+ if (!this.routeData.date) {
+ let yesterday = new Date(this.serverTime);


Why not a const?

dkalinin · 2018-04-27T08:57:38Z

+ display: none;
+ };
+ }
+


Please remove redundant empty line.

dkalinin · 2018-04-27T09:31:04Z

+ return menus.filter(menu => menu.date.getTime() === routeDate.getTime());
+ }
+
+ _getMenuDatesForWeek(menus) {


_getDisplayedDays

dkalinin · 2018-04-27T09:31:26Z

+ return dayOfTheWeek;
+ }
+
+ _getMenuForDay(date) {


getDisplayedMenu

dkalinin · 2018-04-27T09:31:48Z

+
+ _getMenuDatesForWeek(menus) {
+ const weekDates = [];
+ let menuDates = [];


dkalinin · 2018-04-27T09:40:21Z

+ for (let i = 0; i < DAYS_IN_PERIOD; i++) {
+ let tempDate = new Date();
+ tempDate.setDate(this.serverTime.getDate() + i);
+ tempDate.setMonth(this.serverTime.getMonth());


const newDate = new Date(serverTime);
newDate.setDate(serverTime.getDate() + i);

dkalinin · 2018-04-27T09:40:55Z

+ const HOURS_PER_DAY = 86400000;
+ for (let i = 0; i < menuDates.length; i++) {
+ const diff = weekDates[i].getTime() - menuDates[i].getTime();
+ if (diff >= 0 && diff < HOURS_PER_DAY) {


Why do we need this?

PaulAgeyev · 2018-04-27T12:40:17Z

@dkalinin PTAL

…order-component # Conflicts: # client/user/src/mealorder/meal-order-client-app.html # client/user/src/mealorder/menu/meal-order-drawer.html

alexanderkarpetc

See my comments

alexanderkarpetc · 2018-05-02T18:44:56Z

+ * @property {Number} month
+ * @property {Number} year
+ *
+ * @typedef {Array} Categories array contains the category name and dishes


You shouldn't duplicate word array

alexanderkarpetc · 2018-05-02T18:46:07Z

+ * @customElement
+ * @polymer
+ */
+ class MealOrderToolbar extends (Polymer.Element) {


Delete brackets

alexanderkarpetc · 2018-05-02T18:47:10Z

+ * @polymer
+ * @appliesMixin MealOrderApp.ReduxMixin
+ */
+ class MealOrderMenuContainer extends (Polymer.Element) {


Delete brackets

alexanderkarpetc · 2018-05-02T18:50:08Z

+
+ test('should instantiate the `meal-order-drawer` component', function (done) {
+ const elementShadowRoot = component.shadowRoot;
+ const drawer = elementShadowRoot.querySelector("meal-order-drawer");


Those tests look like you are testing polymer

The component is checked for availability.

dkalinin

@PaulAgeyev PTAL

dkalinin · 2018-05-10T08:21:29Z

+ if (!this.routeData.date) {
+ const newDate = new Date(this.serverTime);
+ newDate.setDate(newDate.getDate() - 1);
+ this.set('routeData.date', newDate.toISOString().slice(0, DATE_LENGTH));


.toISOString().slice(0, DATE_LENGTH) is repeated twice in a method.

dkalinin · 2018-05-10T08:27:08Z

+ */
+ selectedDayOfTheWeek: {
+ type: Object,
+ computed: '_getSelectedDayOfTheWeek(routeData.date)'


It should be a String. Also, we have two properties: selectedDay and selectedDayOfTheWeek. It's hard to find the difference by name or js doc. Let's rename selectedDayOfTheWeek to selectedDayName and extend the js doc.

dkalinin · 2018-05-10T08:34:34Z

+ <script>
+ {
+ /**
+ * menu-order is container for the vendors' menus and navigation.


Is a container. Also, please use `menu-order` when mention the tag name. Example: https://github.com/PolymerElements/paper-input/blob/master/paper-input.html
Also, check the js doc of other components.

dkalinin · 2018-05-10T08:36:12Z

+ * @polymer
+ * @appliesMixin MealOrderApp.ReduxMixin
+ */
+ class MealOrderMenuContainer extends Polymer.Element {


It doesn't apply the mixin.

dkalinin · 2018-05-10T08:38:40Z

+ * Selected day.
+ */
+ selectedDate: {
+ type: Object,


It's a String, isn't it?

dkalinin · 2018-05-10T08:49:13Z

+ return date.toISOString().slice(0, DATE_LENGTH);
+ }
+
+ _getDayOfTheWeek(date) {


Let's rename to _getWeekDayName.

dkalinin · 2018-05-10T08:50:24Z

+ * @typedef {Object} Date contains day, month, year
+ * @property {Number} day
+ * @property {Number} month
+ * @property {Number} year


Is it property of type Object or Date?

dkalinin · 2018-05-10T08:52:00Z

+ }
+
+ _getDisplayedDays(menus) {
+ const weekDates = [];


displayedDates

dkalinin · 2018-05-10T08:57:03Z

+
+ <paper-tabs id="paper-tabs" selected="[[selectedDate]]"
+ attr-for-selected='data-key'>
+ <template is="dom-repeat" items="{{displayedDays}}" as="calendar">


It's better to have the day alias instead of calendar.

dkalinin · 2018-05-10T09:18:40Z

+ if (weekDates[i].getTime() === uniqueDates[i].getTime()) {
+ weekDates[i].hasMenu = true;
+ }
+ }


Too much logic to do simple things. All we need is:
A simple array of 8 dates. It is used in top bar to render days.
A Map which is computed from menus array. The key of map is a date, the value is an array of vendor menus by a key date. It's better to use menus.reduce(...) to compute the Map. In order to compute disabled state of day just call map.has(date).

PaulAgeyev · 2018-06-11T20:01:54Z

@dkalinin PTAL

PaulAgeyevand others added 30 commits March 29, 2018 16:27

Add narrow property to app-drawer-layout
e9574ac

Add app-toolbar with iron-pages for menu and history pages
a3a0545

Add app-datepicker to app-drawer for history page
cb34e8d

Add user-cart to app-drawer for menu page
0ced520

Add app-header with text and icons for menu and history pages
2045ff9

Add iron-pages for menu-order and history-order components
e12afad

Add properties for meal-order component
dbf176e

Add actions for meal-order component
5341b3e

Add observers for meal-order component
59ca60a

Add event handler methods for meal-order component
55d2ada

Add docs for meal-order component
f9a0b5a

Add CSS for meal-order component
50ccdc8

Add component imports for the "meal-order" component
cd03ce8

Create menu-order component
23dcad9

Add routing for the menu-order component
3302408

Add paper-tabs for the menu-order component
4a113ed

Add iron-pages with vendor-menus component for the menu-order c…
c874805
…omponent

Add observer with _pageChanged' method for the menu-order` component
f317d41

Add properties for the menu-order component
c9f3c41

Add actions for the menu-order component
e1b02b2

Add connectedCallback with menuForDay action for the menu-order…
e1098d7
… component

Add handler for paper-tabs
01af2ba

Add event handler for the dish-added event
8252689

Add docs for the menu-order component
e870564

Add component imports for the "menu-order" component
b8d2244

Add CSS for the menu-order component
c3adee3

Create vendor-menus component
50727ef

Add iron-list of menus
d5eadcb

Add search-bar
0f33be7

Add styles for desktop and mobile
8ed7538

PaulAgeyev added 3 commits April 26, 2018 18:19

Add docs
7acf542

Remove the order property from the meal-order-drawer component
2f2d198

Remove unnecessary actions
3c7accf

dkalinin requested changes Apr 26, 2018
View reviewed changes

PaulAgeyev added 4 commits April 26, 2018 21:50

Extract the app-toolbar to the component
901952b

Add drawer toggle and docs for the meal-order-toolbar
ed7c1d7

Apply two way data binding for routing
b88632e

Remove unnecessary semicolon
1e342a0

dkalinin requested changes Apr 27, 2018
View reviewed changes

Apply the date format for the app-datepicker
fee38f0

PaulAgeyev added 4 commits April 27, 2018 16:14

Add tests
d607fcf

Add tests using MealOrderApp namespace
846a757

Refactor tests
13d2241

Change the server time to a Date type
3bbb00d

PaulAgeyev requested review from alexanderkarpetc and removed request for alexanderkarpetc April 30, 2018 13:41

PaulAgeyev added 4 commits May 1, 2018 13:32

Remove unnecessary component imports
2151194

Merge remote-tracking branch 'origin/menu-order-component' into menu-…
3d9253c
…order-component # Conflicts: # client/user/src/mealorder/meal-order-client-app.html # client/user/src/mealorder/menu/meal-order-drawer.html

Remove unnecessary dependencies from the bower
206cce1

Reformat code
7c48746

alexanderkarpetc reviewed May 2, 2018
View reviewed changes

Refactor docs
65ac411

dkalinin requested changes May 10, 2018
View reviewed changes

PaulAgeyev added 3 commits June 11, 2018 22:46

Improve implementation to select the date for the order
45b6556

Add docs
3287aaf

Rename property for an existing menu
defdc4d

PaulAgeyev added 2 commits June 12, 2018 11:20

Add missing brackets
6478826

Create a Map with a list of existing menus
5441211

		@@ -19,9 +19,8 @@
		-->
		<link rel="import" href="shell/meal-order-app-namespace.html">

Uh oh!

Conversation

PaulAgeyev commented Mar 29, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkalinin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkalininApr 26, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkalinin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaulAgeyev commented Apr 27, 2018

Uh oh!

alexanderkarpetc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkalinin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

PaulAgeyev commented Mar 29, 2018•
edited
Loading

dkalininApr 26, 2018•
edited
Loading

dkalininMay 10, 2018•
edited
Loading

dkalininMay 10, 2018•
edited
Loading