Feature/favorite rooms #422

nmate980829 · 2020-09-02T21:51:26Z

Add feature favorite rooms. #332

triszt4n

Nice code, clever solution, loved the roomTemplate!

triszt4n · 2020-09-08T15:06:19Z

public/js/favorite.js

+function addFavorite(room) {
+	if (!room) {
+		displayMessage('danger', err)
+  	} else {
+		fetch('/favorites', { 
+			method: 'POST',  
+			headers: {
+      			'Content-Type': 'application/json'
+    		},
+			body: JSON.stringify({ room }), 
+		})
+    .then(async (res) => {
+		  switch (res.status) {
+		  	case 201:
+			    location.reload()
+          break
+		  	case 401:
+          displayMessage('danger', UNAUTHORIZED_MESSAGE)
+					break
+				case 403:
+        	displayMessage('danger', FORBIDDEN_MESSAGE)
+        	break
+		  	case 404:
+        	data = await res.json()
+        	displayMessage('danger', data.message)
+        	break
+		  }
+      })
+      .catch((err) => displayMessage('danger', err))
+  }
+}


Could you please correct the indentation in this file? Especially here.

We like to indent using two spaces.
Consistency (even with indentation) is very nice for the developer's eyes. 😄

Sorry for that. My VS code is automatically indenting with tabs, and resets the settings after every restart. I will run a lint with --fix option. That will do it.

triszt4n · 2020-09-08T15:13:42Z

views/room/roomTemplate.pug

+  .column.notification(class=`${!group ? 'is-success' : (!group.doNotDisturb? 'is-warning' : 'is-danger')}`)
+    .columns.is-vcentered.is-gapless
+      .column.is-narrow
+        span.icon.is-medium.has-text-warning.mr-2(onClick=`${favorite? `deleteFavorite(${room})` : `addFavorite(${room})`}`)


Maybe create a .is-clickable class in additions.scss, and put it onto this span as well?

Recommendation:

.is-clickable { cursor: pointer; }

public/js/favorite.js

OmTheTurtle · 2020-09-10T10:04:29Z

src/components/favorites/favorite.service.ts

+    if (
+      req.params.id &&
+      favorites &&
+      favorites.findIndex(element => element.room == parseInt(req.params.id)) >= 0


Can we use .some() here as well?

Sorry, I forgot to replace it. At first I used this method for some mysterious reason, and then changed it.

OmTheTurtle · 2020-09-10T10:06:55Z

src/components/favorites/favorite.service.ts

+    res.sendStatus(404)
+  }
+})
+


There are some leftover whitespaces here.

OmTheTurtle · 2020-09-10T10:11:58Z

src/components/rooms/room.routes.ts

+          favorite: favorites.some(el => el.room == room.floor)
+        }
+      }) 
+    if (user.floor && user.floor >= 3 && user.floor <= 18) {


I think we can completely remove the floor from the use model and it's mentions from the application as the favorites extends this functionality.

Should I do it in this PR, or should I introduce a new issue for it?

Do it in here since this feature makes the current functionality obsolete.

OmTheTurtle · 2020-10-14T10:58:32Z

views/room/index.pug

-        - const group = busyRooms.find(it => it.id === room)
-        if !group.doNotDisturb
+    if user && rooms.some(el => el.favorite)
+      section.mt-5.mb-5 A kedvenceid:


I'd remove this text, it doesn't really add any extra information.

OmTheTurtle · 2020-10-14T10:59:43Z

views/room/index.pug

-        else
+            +roomTemplate(room.floor, room.favorite, room.busy)  
+    if rooms.some(el => !el.favorite)
+      section.mt-5.mb-5 Szintek:


I'd change this to a simple <hr /> or something similar.

OmTheTurtle · 2020-10-14T11:00:06Z

views/user/show.pug

@@ -4,14 +4,6 @@ block content
  h1.title= userToShow.name
  if user.id === userToShow.id
    h2.subtitle.my-5 Személyes beállítások


You can remove this line too, as currently there are no settings on the profile page.

OmTheTurtle · 2020-12-19T23:48:40Z

There are some conflicting files with the latest code in master, please rebase your branch to resolve it.

…xt into feature/favorite-rooms

triszt4n · 2021-04-09T19:32:47Z

👀

nmate980829 · 2021-04-14T19:23:02Z

I've waited for the new frontend with this pr, and rebased it again, but I think I'd rather delete the branch. It's kinda messy, so I think I'll just rewrite it if thats okay?

OmTheTurtle · 2021-04-14T21:50:25Z

Whatever is easier for you :) Just don't forget to delete the remote branch too.

nmate980829 requested review from OmTheTurtle and triszt4n and removed request for OmTheTurtle September 2, 2020 21:51

nmate980829 added 2 commits September 7, 2020 20:09

add feature favorite rooms #332

7181346

fix add eslint comment

fc9daa4

nmate980829 force-pushed the feature/favorite-rooms branch from 536fdaf to fc9daa4 Compare September 7, 2020 18:11

triszt4n reviewed Sep 8, 2020

View reviewed changes

triszt4n linked an issue Sep 8, 2020 that may be closed by this pull request

Customizable room order on rooms page #332

Open

nmate980829 added 2 commits September 8, 2020 19:02

fix indentation

2fa386e

make favorite button clickable

d2c07e3

nmate980829 requested a review from triszt4n September 8, 2020 17:12

OmTheTurtle reviewed Sep 10, 2020

View reviewed changes

public/js/favorite.js Show resolved Hide resolved

OmTheTurtle reviewed Sep 10, 2020

View reviewed changes

src/components/favorites/favorite.service.ts Outdated

res.sendStatus(404)

}

})

Copy link

Member

OmTheTurtle Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some leftover whitespaces here.

OmTheTurtle reviewed Sep 10, 2020

View reviewed changes

nmate980829 added 2 commits October 13, 2020 22:02

remove feature floor

ce5c8a1

Merge branch 'master' into feature/favorite-rooms

a22c825

nmate980829 requested a review from OmTheTurtle October 13, 2020 20:28

OmTheTurtle requested changes Oct 14, 2020

View reviewed changes

add requested changes

af90ffc

nmate980829 requested a review from OmTheTurtle December 19, 2020 21:12

nmate980829 added 7 commits February 6, 2021 16:11

add feature favorite rooms #332

8b192d0

fix add eslint comment

468bff1

fix indentation

846c055

conflict

87131d1

remove feature floor

a5d2f38

add requested changes

86c4dd8

Merge branch 'feature/favorite-rooms' of github.com:kir-dev/tanulo-ne…

4e8cc06

…xt into feature/favorite-rooms

nmate980829 closed this Jun 9, 2021

nmate980829 deleted the feature/favorite-rooms branch June 9, 2021 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/favorite rooms #422

Feature/favorite rooms #422

nmate980829 commented Sep 2, 2020

triszt4n left a comment

triszt4n Sep 8, 2020

nmate980829 Sep 8, 2020

triszt4n Sep 8, 2020

OmTheTurtle Sep 10, 2020

nmate980829 Oct 3, 2020

OmTheTurtle Sep 10, 2020

OmTheTurtle Sep 10, 2020 •

edited

Loading

nmate980829 Oct 3, 2020 •

edited

Loading

OmTheTurtle Oct 3, 2020

OmTheTurtle Oct 14, 2020

OmTheTurtle Oct 14, 2020

OmTheTurtle Oct 14, 2020

OmTheTurtle commented Dec 19, 2020

triszt4n commented Apr 9, 2021

nmate980829 commented Apr 14, 2021

OmTheTurtle commented Apr 14, 2021

Feature/favorite rooms #422

Feature/favorite rooms #422

Conversation

nmate980829 commented Sep 2, 2020

triszt4n 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmTheTurtle Sep 10, 2020 • edited Loading

Choose a reason for hiding this comment

nmate980829 Oct 3, 2020 • edited Loading

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

OmTheTurtle commented Dec 19, 2020

triszt4n commented Apr 9, 2021

👀

nmate980829 commented Apr 14, 2021

OmTheTurtle commented Apr 14, 2021

OmTheTurtle Sep 10, 2020 •

edited

Loading

nmate980829 Oct 3, 2020 •

edited

Loading