Serraniel/AniwatchPlus
Serraniel/AniwatchPlus
1
0
Fork 0
Code Issues 9 Pull requests 8 Packages Projects Releases 10 Wiki Activity

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
Conversation 7 Commits 25 Files changed 4 +40 -31
4 changed files with 40 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 1962547d75 - Show all commits

35
src/javascript/enhancements/notifications.js
View file

@ -1,22 +1,26 @@
import * as core from '../utils/aniwatchCore';
import * as helper from '../utils/helpers';
let __notificationCount = ''; let __notificationCount = '';
runAfterLoad(() => { export function init() {
retrieveLoginStatus(); core.runAfterLoad(() => {
__notificationCount = getNotificationCount(); retrieveLoginStatus();
displayNotificationsInTitle(); __notificationCount = getNotificationCount();
}, ".*"); displayNotificationsInTitle();
kaffem commented 2020-09-14 00:24:10 +02:00 (Migrated from github.com)
Review
Copy link

Getting the NotificationCount on every pushState event increases the chance of it failing. With this solution it fails for every item of the Community and Anime dropdownmenus. With the old solution it failed when inserting the Count into the title for the stats, jobs, seasonal, airing, random, profile, watchlist and notifications. 10 vs 8 pathnames where it doesn't work.
Furthermore with this solution we run into problems on the profile pathname:
chrome_yyDTOAsutx
We are adding the NotificationCount to the old count whenever we switch from one tab under profile to another; this did not happen with the old solution.

This seems to be related to #42 tho, if you enable Low-end mobile and check the pathnames after visting /home first, you'll see the notification count appear for a while and then disappear once the site is fully shown. With the new solution its definitely because of #42; we are trying to access the menu while the page isn't fully loaded.

Getting the NotificationCount on every pushState event increases the chance of it failing. With this solution it fails for every item of the Community and Anime dropdownmenus. With the old solution it failed when inserting the Count into the title for the stats, jobs, seasonal, airing, random, profile, watchlist and notifications. 10 vs 8 pathnames where it doesn't work. Furthermore with this solution we run into problems on the profile pathname: ![chrome_yyDTOAsutx](https://user-images.githubusercontent.com/29717789/93030124-fe0ef180-f620-11ea-99b3-151a7ed6a99b.png) We are adding the NotificationCount to the old count whenever we switch from one tab under profile to another; this did not happen with the old solution. This seems to be related to #42 tho, if you enable Low-end mobile and check the pathnames after visting /home first, you'll see the notification count appear for a while and then disappear once the site is fully shown. With the new solution its definitely because of #42; we are trying to access the menu while the page isn't fully loaded.
Serraniel commented 2020-09-16 21:36:59 +02:00 (Migrated from github.com)
Review
Copy link

Do you remember how exactly you produced that bug with the number written in the title multiple times? I haven´t seen that yet.

But something else I noticed was the fact, the title seems to be updated by aniwatch after the blue loading bar on the top disappears. This should be investigated as a new opportunity to decide when we update the title and also would fix the current issue, that we just guess a random number to wait until location change before we update the title, which doesn´t always works as the following gif shows:
2020-09-16_21-32-11-463

Do you remember how exactly you produced that bug with the number written in the title multiple times? I haven´t seen that yet. But something else I noticed was the fact, the title seems to be updated by aniwatch after the blue loading bar on the top disappears. This should be investigated as a new opportunity to decide when we update the title and also would fix the current issue, that we just guess a random number to wait until location change before we update the title, which doesn´t always works as the following gif shows: ![2020-09-16_21-32-11-463](https://user-images.githubusercontent.com/8461282/93383974-b7a9d480-f864-11ea-9ec9-8b560256357b.gif)
kaffem commented 2020-09-18 00:30:46 +02:00 (Migrated from github.com)
Review
Copy link

Yeah thats what I meant. Looks like we should not only wait for the popState but also for onload once their preloader is fixed, or wait for the loading bar to disappear(?)

Yeah thats what I meant. Looks like we should not only wait for the popState but also for onload once their preloader is fixed, or wait for the loading bar to disappear(?)
Serraniel commented 2020-10-25 10:57:50 +01:00 (Migrated from github.com)
Review
Copy link

I introduced another observer, which will execute after the blue loading bar in the top disappears. This error seems not to appear any more with that specific change.

I introduced another observer, which will execute after the blue loading bar in the top disappears. This error seems not to appear any more with that specific change.
Serraniel commented 2020-10-25 11:00:43 +01:00 (Migrated from github.com)
Review
Copy link
  • There still is the problem, that some pages have a different loading method where the menu isn´t available directly in the DOM.
- [x] There still is the problem, that some pages have a different loading method where the menu isn´t available directly in the DOM.
Serraniel commented 2020-10-25 11:29:42 +01:00 (Migrated from github.com)
Review
Copy link

I was able to fix this. We started our scripts after the preloader disappeared. We now also wait for the document.readyState to be "complete" so we can ensure everything is loaded correctly, as the preloader disappears to early on some pages.

I was able to fix this. We started our scripts after the preloader disappeared. We now also wait for the `document.readyState` to be `"complete"` so we can ensure *everything* is loaded correctly, as the preloader disappears to early on some pages.
}, ".*");
runAfterPathnameChange(() => { core.runAfterPathnameChange(() => {
displayNotificationsInTitle(); displayNotificationsInTitle();
}, ".*"); }, ".*");
}
function getNotificationCount() { function getNotificationCount() {
if (isLoggedIn) { if (core.isLoggedIn()) {
let menuUserText = document.getElementById('materialize-menu-dropdown').innerText.split('\n')[4]; let menuUserText = document.getElementById('materialize-menu-dropdown').innerText.split('\n')[4];
let notificationCount = menuUserText.split("")[6]; let notificationCount = menuUserText.split('')[6];
console.log(notificationCount);
// If there are no notifications // If there are no notifications
if (Number.isNaN(parseInt(notificationCount)) || typeof notificationCount === 'undefined') { if (Number.isNaN(parseInt(notificationCount)) || !helper.assigned(notificationCount)) {
console.warn("NaN or undefined"); console.warn("NaN or undefined");
return ``; // Otherwise displayNotificationsInTitle() throws undefined again return ``; // Otherwise displayNotificationsInTitle() throws undefined again
} }
@ -29,10 +33,9 @@ function getNotificationCount() {
function displayNotificationsInTitle() { function displayNotificationsInTitle() {
console.log(__notificationCount); console.log(__notificationCount);
if (typeof __notificationCount === 'undefined') { if (!helper.assigned(__notificationCount)) {
console.error("NoTiFiCaTiOnCoUnT uNdEfInEd!"); console.error("NoTiFiCaTiOnCoUnT uNdEfInEd!");
} } else {
else {
document.title = __notificationCount + document.title; document.title = __notificationCount + document.title;
} }
} }

2
src/javascript/index.js
View file

@ -7,6 +7,7 @@ import { initHelpers } from './utils/helpers';
// enhancements // enhancements
import { init as animeRequests } from './enhancements/animeRequests'; import { init as animeRequests } from './enhancements/animeRequests';
import { init as lists } from './enhancements/lists'; import { init as lists } from './enhancements/lists';
import { init as notifications } from './enhancements/notifications';
import { init as quickSearch } from './enhancements/quickSearch'; import { init as quickSearch } from './enhancements/quickSearch';
// core // core
@ -18,4 +19,5 @@ initHelpers();
// enhancements // enhancements
animeRequests(); animeRequests();
lists(); lists();
notifications();
quickSearch(); quickSearch();

18
src/javascript/utils/aniwatchCore.js
View file

@ -72,8 +72,20 @@ function awaitPageLoaded() {
}, 100); }, 100);
} }
kaffem commented 2020-09-14 00:29:53 +02:00 (Migrated from github.com)
Review
Copy link

We should rename this, if we keep using the pushState event.

We should rename this, if we keep using the pushState event.
Serraniel commented 2020-09-16 21:32:23 +02:00 (Migrated from github.com)
Review
Copy link

What would you suggest? Maybe location change? I think each pushstate also always could be a location change maybe?

What would you suggest? Maybe location change? I think each pushstate also always could be a location change maybe?
kaffem commented 2020-09-18 00:32:34 +02:00 (Migrated from github.com)
Review
Copy link
export function runAfterLocationChange(func, pattern = '.*') {

would be fine imo

```suggestion export function runAfterLocationChange(func, pattern = '.*') { ``` would be fine imo
Serraniel commented 2020-10-25 10:58:41 +01:00 (Migrated from github.com)
Review
Copy link

Renamed this with commit a840f644b7

Renamed this with commit a840f644b723cb885232f4184698db2a27bc5f7d
function runAfterPathnameChange(func, pattern = '.*') { export function runAfterPathnameChange(func, pattern = '.*') {
__afterPathnameChangeScripts.push({ "function": func, "pattern": pattern}); __afterPathnameChangeScripts.push({ "function": func, "pattern": pattern });
}
export function isLoggedIn() {
let menu = document.getElementById('materialize-menu-dropdown');
let result = true;
menu.innerText.split('\n').forEach(item => {
if (item === 'Login') {
result = false;
return;
}
});
} }
let locationPath = location.pathname; let locationPath = location.pathname;
@ -90,4 +102,4 @@ function awaitPathnameChange() {
script.function(); script.function();
} }
}) })
} }

16
src/javascript/utils/helpers.js
View file

@ -18,6 +18,10 @@ export function onReady(fn) {
} }
} }
export function assigned(obj) {
return !(typeof obj === 'undefined' || obj === null);
}
function handleKeyDown(event) { function handleKeyDown(event) {
handleKeyToggle(event, true); handleKeyToggle(event, true);
} }
@ -32,16 +36,4 @@ function handleKeyToggle(event, isPressed) {
} else if (event.key === 'Control') { } else if (event.key === 'Control') {
isCtrlPressed = isPressed; isCtrlPressed = isPressed;
} }
}
export function retrieveLoginStatus() {
let menu = document.getElementById('materialize-menu-dropdown');
let menuItem = menu.innerText.split('\n')[4];
if (menuItem === 'Login') {
isLoggedIn = false;
console.log(isLoggedIn);
} else if (menuItem.includes('User')) {
isLoggedIn = true;
console.log(isLoggedIn);
}
} }