Feature/#19 notifications tab title #31

Merged
kaffem merged 25 commits from feature/#19-notifications-tab-title into develop 2020-10-28 06:46:24 +01:00
Showing only changes of commit 05a4a730ec - Show all commits

View file

@ -6,5 +6,5 @@ function displayNotificationsInTitle(){
Serraniel commented 2020-08-05 11:46:20 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image

There probably also has to be a check if user is logged in and if there even is a notification I think.
You may try getting it via regular expression:

menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' 
Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) There probably also has to be a check if user is logged in and if there even is a notification I think. You may try getting it via regular expression: ```js menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' ```
Serraniel commented 2020-08-05 11:48:30 +02:00 (Migrated from github.com)
Review

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.
kaffem commented 2020-08-05 15:54:38 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image
Yeah the 3 is the correct index here, your split is missing another space.
chrome_8SQg6ZZOCH

> > > Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: > ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) Yeah the 3 is the correct index here, your split is missing another space. ![chrome_8SQg6ZZOCH](https://user-images.githubusercontent.com/29717789/89420569-2eff2b00-d733-11ea-95e1-8735ac78c4eb.png)
kaffem commented 2020-08-05 15:57:54 +02:00 (Migrated from github.com)
Review

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether
kaffem commented 2020-08-20 17:52:01 +02:00 (Migrated from github.com)
Review

There probably also has to be a check if user is logged in and if there even is a notification I think.

9fe2668c3a adds checks for logged in + notifications

> There probably also has to be a check if user is logged in and if there even is a notification I think. 9fe2668c3afec7a4b100cffc9ae722d1baa13fe7 adds checks for logged in + notifications
Serraniel commented 2020-08-05 11:46:20 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image

There probably also has to be a check if user is logged in and if there even is a notification I think.
You may try getting it via regular expression:

menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' 
Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) There probably also has to be a check if user is logged in and if there even is a notification I think. You may try getting it via regular expression: ```js menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' ```
Serraniel commented 2020-08-05 11:48:30 +02:00 (Migrated from github.com)
Review

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.
kaffem commented 2020-08-05 15:54:38 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image
Yeah the 3 is the correct index here, your split is missing another space.
chrome_8SQg6ZZOCH

> > > Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: > ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) Yeah the 3 is the correct index here, your split is missing another space. ![chrome_8SQg6ZZOCH](https://user-images.githubusercontent.com/29717789/89420569-2eff2b00-d733-11ea-95e1-8735ac78c4eb.png)
kaffem commented 2020-08-05 15:57:54 +02:00 (Migrated from github.com)
Review

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether
kaffem commented 2020-08-20 17:52:01 +02:00 (Migrated from github.com)
Review

There probably also has to be a check if user is logged in and if there even is a notification I think.

9fe2668c3a adds checks for logged in + notifications

> There probably also has to be a check if user is logged in and if there even is a notification I think. 9fe2668c3afec7a4b100cffc9ae722d1baa13fe7 adds checks for logged in + notifications
let menu = document.getElementById('materialize-menu-dropdown');
let menuDropdowns = Array.from(menu.querySelectorAll('ul.dropdown')).slice(-1)[0];
let notificationText = menuDropdowns.innerText.split(" ")[3];
document.querySelector('head > title').insertAdjacentText('afterbegin', notificationText.split(" ")[1] + ' ');
Serraniel commented 2020-08-05 11:46:20 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image

There probably also has to be a check if user is logged in and if there even is a notification I think.
You may try getting it via regular expression:

menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' 
Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) There probably also has to be a check if user is logged in and if there even is a notification I think. You may try getting it via regular expression: ```js menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' ```
Serraniel commented 2020-08-05 11:48:30 +02:00 (Migrated from github.com)
Review

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.
kaffem commented 2020-08-05 15:54:38 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image
Yeah the 3 is the correct index here, your split is missing another space.
chrome_8SQg6ZZOCH

> > > Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: > ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) Yeah the 3 is the correct index here, your split is missing another space. ![chrome_8SQg6ZZOCH](https://user-images.githubusercontent.com/29717789/89420569-2eff2b00-d733-11ea-95e1-8735ac78c4eb.png)
kaffem commented 2020-08-05 15:57:54 +02:00 (Migrated from github.com)
Review

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether
kaffem commented 2020-08-20 17:52:01 +02:00 (Migrated from github.com)
Review

There probably also has to be a check if user is logged in and if there even is a notification I think.

9fe2668c3a adds checks for logged in + notifications

> There probably also has to be a check if user is logged in and if there even is a notification I think. 9fe2668c3afec7a4b100cffc9ae722d1baa13fe7 adds checks for logged in + notifications
document.title = notificationText.split(" ")[1] + ' ' + document.title;
Serraniel commented 2020-08-05 11:46:20 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image

There probably also has to be a check if user is logged in and if there even is a notification I think.
You may try getting it via regular expression:

menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' 
Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) There probably also has to be a check if user is logged in and if there even is a notification I think. You may try getting it via regular expression: ```js menuDropdowns.querySelector('li a[href="notification"]')?.innerText?.match('\\d+')[0] ?? '' ```
Serraniel commented 2020-08-05 11:48:30 +02:00 (Migrated from github.com)
Review

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.

We should also insert a space after the number (at least the code I put in the other comment is without space). Also only add the space if there are notifications to insert.
kaffem commented 2020-08-05 15:54:38 +02:00 (Migrated from github.com)
Review

Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think:
image
Yeah the 3 is the correct index here, your split is missing another space.
chrome_8SQg6ZZOCH

> > > Is 3 the correct index here? I just executed this via console and it wasn´t working correctly I think: > ![image](https://user-images.githubusercontent.com/8461282/89397194-d5d1d000-d70f-11ea-804b-f4ca40b9c65d.png) Yeah the 3 is the correct index here, your split is missing another space. ![chrome_8SQg6ZZOCH](https://user-images.githubusercontent.com/29717789/89420569-2eff2b00-d733-11ea-95e1-8735ac78c4eb.png)
kaffem commented 2020-08-05 15:57:54 +02:00 (Migrated from github.com)
Review

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether

Right now there is a space added after the number in line 16; however I'll take a look into retrieving the notificationCount via regex since its not always working on pathnames like /watch2gether
kaffem commented 2020-08-20 17:52:01 +02:00 (Migrated from github.com)
Review

There probably also has to be a check if user is logged in and if there even is a notification I think.

9fe2668c3a adds checks for logged in + notifications

> There probably also has to be a check if user is logged in and if there even is a notification I think. 9fe2668c3afec7a4b100cffc9ae722d1baa13fe7 adds checks for logged in + notifications
}