Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial commit for show badge issue #149

Merged
merged 23 commits into from
Mar 20, 2018
Merged

initial commit for show badge issue #149

merged 23 commits into from
Mar 20, 2018

Conversation

khaydarov
Copy link
Member

@khaydarov khaydarov commented Feb 27, 2018

close #72

@@ -22,6 +23,8 @@ class NotesController {
* Setup event handlers
*/
constructor() {
this.visits = new Visits();
Copy link
Member

Choose a reason for hiding this comment

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

комментарий

@@ -96,6 +102,18 @@ class NotesController {
let list = new NotesList(folderId);
let notesInFolder = await list.get();

await this.visits.syncVisits();
Copy link
Member

Choose a reason for hiding this comment

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

комментарий

* set up note visits
*/
notesInFolder.forEach( async (note) => {
if ( this.visits.visitedNotes[note._id] >= note.dtModify ) {
Copy link
Member

Choose a reason for hiding this comment

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

this.visits.getDate(note._id)

Copy link
Member

Choose a reason for hiding this comment

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

не обрабатывается отсутствие статьи в visits

Copy link
Member

Choose a reason for hiding this comment

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

перенести в модель NotesList в свойство isSeen

*/
notesInFolder.forEach( async (note) => {
if ( this.visits.visitedNotes[note._id] >= note.dtModify ) {
note.visited = true;
Copy link
Member

Choose a reason for hiding this comment

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

note.is_read = true

const db = require('../utils/database'),
Time = require('../utils/time');

class Visits {
Copy link
Member

Choose a reason for hiding this comment

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

SeenStateObserver

/**
* set up note visits
*/
notesInFolder.forEach( async (note) => {
Copy link
Member

Choose a reason for hiding this comment

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

подгружать индикаторы isSeen отдельным запросом после открытия папки, передав ids статей

* "once" automatically removes listener
*/
window.ipcRenderer.send('notes - seen', { noteIds });
window.ipcRenderer.once('notes - seen', (event, {data}) => {
Copy link
Member

Choose a reason for hiding this comment

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

надо описать {data}

});

ipcMain.on('note - delete', (event, {id}) => {
this.deleteNote(id, event);
});

ipcMain.on('notes - seen', async (event, {noteIds}) => {
let seenNotes = await this.SeenStateObserver.getSeenNotes(noteIds);
Copy link
Member

Choose a reason for hiding this comment

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

вынести логику в обработчик this.markAsSeen();

ipcMain.on('note - save', (event, {note}) => {
this.saveNote(note, event);
this.SeenStateObserver.touch(note.data.id);
Copy link
Member

Choose a reason for hiding this comment

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

перенести логику в обработчик this.saveNote

@@ -32,11 +36,21 @@ class NotesController {

ipcMain.on('note - get', (event, {id}) => {
this.getNote(id, event);

// set note as visited
this.SeenStateObserver.touch(id);
Copy link
Member

Choose a reason for hiding this comment

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

перенести логику в обработчик

* @todo asynchronous notes load
*/
this.notesListWrapper = document.querySelector('[name="js-folder-notes-menu"]');

Copy link
Member

Choose a reason for hiding this comment

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

зачем убран todo? он еще актуален

setNoteSeenStatus() {
let noteIds = [];

this.notes.forEach( (note) => {
Copy link
Member

Choose a reason for hiding this comment

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

let noteIds = this.notes.map(note => note._id);


/**
* @type {Object} data - "note - seen" event responses object of "seen" states
* Ex: data[ noteId ] = lastSeen - timestamp of last seen
Copy link
Member

Choose a reason for hiding this comment

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

эти две строчки комментов оформлены не по jsdoc

@@ -78,6 +85,10 @@ class NotesController {
note: newNote,
isRootFolder: !noteData.folderId
});

// make "seen" edited note
Copy link
Member

@neSpecc neSpecc Mar 15, 2018

Choose a reason for hiding this comment

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

mark edited Note as seen

@@ -149,6 +163,19 @@ class NotesController {
event.returnValue = false;
}
}

/**
* Get's information about note visits and emits the event
Copy link
Member

Choose a reason for hiding this comment

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

about note visits

about seen-state of passed notes

async markAsSeen(event, noteIds) {
let seenNotes = await this.seenStateObserver.getSeenNotes(noteIds);
event.sender.send('notes - seen', {
data: seenNotes
Copy link
Member

Choose a reason for hiding this comment

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

data не лучшее название в данном случае (я долго пытался понять код клиента), лучше назвать seenIds.

Copy link
Member

Choose a reason for hiding this comment

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

и вообще data — всегда плохое название

@@ -0,0 +1,70 @@
/**
* @module SeenStateObserver
* Checks seen notes or makes notes "seen"
Copy link
Member

Choose a reason for hiding this comment

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

Manipulates seen-state of Notes

* @class SeenStateObserver
* Class can "touch" note (mark as seen) or get information about visited notes
*
* @property this.getSeenNotes {Function} - returns information about passed note
Copy link
Member

Choose a reason for hiding this comment

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

this не нужно. тип ставится перед названием 😟


/**
* Sets new last seen time to mark note as read
* @param noteId {Number} - note id
Copy link
Member

Choose a reason for hiding this comment

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

тип перед названием

let response = {};

notes.forEach( (note) => {
response[note.noteId] = note.lastSeen;
Copy link
Member

Choose a reason for hiding this comment

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

есть подозрение, что у тебя не работает линтер

lastSeen : Time.now
};

await db.update(db.VISITS, query, {
Copy link
Member

Choose a reason for hiding this comment

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

из параметров нужно вынести объекты в переменные

Copy link
Member Author

@khaydarov khaydarov Mar 20, 2018

Choose a reason for hiding this comment

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

зачем создавать лишние переменные и выделять для них память, когда после вызова тут же уничтожаются

Copy link
Member

@neSpecc neSpecc left a comment

Choose a reason for hiding this comment

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

👍

@neSpecc neSpecc merged commit 595145d into version2.0 Mar 20, 2018
@neSpecc neSpecc deleted the show-badge branch March 20, 2018 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show count updates label
3 participants