feat: display the note parents structure #275

dependentmadani · 2024-11-07T13:26:59Z

Problem:

The issue is that when accessing a note, you would not know the note parent hierarchy, or the note parent of the actual note you are accessing.

Solution:

As describe in issue #256, we will be displaying the parent note structure at the top left of the note, following the tasks required to insure that the display of the note structure would be efficient.

Resolves #256

e11sy · 2024-11-07T13:43:41Z

src/application/services/useNote.ts

+   *
+   * Actual note by default
+   */
+  const noteParents = ref<Note[]>([]);


i think that making noteParents reactive is a bad thing to do
We can update them only by request to api /GET note so i can't see the case, when we update noteParents and imidiately change displayed content somewhere

still actual

e11sy · 2024-11-07T13:46:32Z

src/application/services/useNote.ts

+    if (currentId.value === null) {
+      throw new Error('note id is not defined');
+    }
+    let presentationFormat = '';
+
+    for (let value of noteParents.value) {
+      presentationFormat += getTitle(value.content) + ' > ';
+    }
+    presentationFormat += noteTitle.value;
+
+    return presentationFormat;


parentNote title should be clickable so just merge titles into one string is not a solution.

Also can't see the case with RootNoteTitle > ... > CurrentNote

I think we can use anchor tag <a> can solve the merge of titles into one string.

it's better to do it in a component

neSpecc · 2024-11-08T14:52:49Z

src/application/services/useNote.ts

@@ -265,6 +273,23 @@ export default function (options: UseNoteComposableOptions): UseNoteComposableSt
    parentNote.value = undefined;
  }

+  /**
+   * Format the received note parents into presentation format


Description is not clear

…n useNoteComposableState interface

src/application/services/useNote.ts

…work to be done

…pplied, still need work to be done

neSpecc · 2024-12-14T12:35:17Z

src/application/services/useNote.ts

+  /**
+   * Returns an array of Note objects representing the formatted note parents.
+   */
+  formatNoteParents: () => Note[];


from the "composable" pattern its better to export "noteParents" object instead

neSpecc · 2024-12-14T12:36:15Z

src/application/services/useNote.ts

+  /**
+   * Reform the received note parents from api into presentation format.
+   * @returns An array of Note objects representing the formatted note parents.
+   * @throws {Error} If the note id is not defined.
+   */


I dont really understand what this method is doing. Could you please improve jsdoc to make it more clear? Why API format is not suitable?

neSpecc · 2024-12-14T12:37:46Z

src/presentation/pages/Note.vue

+            v-for="(parent, index) in displayedParents"
+            :key="parent.id"
+          >
+            <a @click="handleParentClick(parent)">{{ newGetTitle(parent.content) }}</a>


use RouterLink instead of a

neSpecc · 2024-12-14T12:37:53Z

src/presentation/pages/Note.vue

-            : t('note.lastEdit') + ' ' + 'a few seconds ago'
-        }}
+        <div v-if="noteParents.length">
+          <span


span seems redunant

neSpecc · 2024-12-14T12:38:26Z

src/presentation/pages/Note.vue

+ * @param {string} parent.id - The ID of the parent note
+ */
+function handleParentClick(parent: { id: string }): void {
+  if (parent.id !== 'ellipsis') {


seems like a magic string

neSpecc · 2024-12-14T12:38:47Z

src/presentation/pages/Note.vue

+ * @param {OutputData | { title: string }} content - The content of the note
+ * @returns {string} - The title of the note
+ */
+function newGetTitle(content: OutputData | { title: string }): string {


we already have such a method

neSpecc · 2024-12-17T20:31:41Z

src/application/services/useNote.ts

+      presentationFormat.push({
+        id: currentId.value,
+        content: note.value?.content as NoteContent,
+      });


why you are adding current note as a parent? I don't think that it is a good idea

neSpecc · 2024-12-17T20:32:26Z

src/application/services/useNote.ts

@@ -265,6 +278,35 @@ export default function (options: UseNoteComposableOptions): UseNoteComposableSt
    parentNote.value = undefined;
  }

+  /**
+   * Reform the array of note parents by adding the actual note id and content.


still don't understand the problem. API returns id and content, why do we need to change them?

neSpecc · 2024-12-17T20:32:35Z

src/presentation/pages/Note.vue

-            ? t('note.lastEdit') + ' ' + getTimeFromNow(note.updatedAt)
-            : t('note.lastEdit') + ' ' + 'a few seconds ago'
-        }}
+        <div>


div seems redundant

neSpecc · 2024-12-17T20:33:09Z

src/presentation/pages/Note.vue

+            v-for="(parent, index) in displayedParents"
+            :key="index"
+            :to="{ path: `/note/${parent.id ? parent.id : noteId}` }"
+            @click.prevent="handleParentClick($event, parent)"


probably you don't need to handle click manually since you are using router link

neSpecc · 2024-12-17T20:33:22Z

src/presentation/pages/Note.vue

+ *
+ * @param {number} parentId - The ID of the parent note to navigate to
+ */
+function navigateToParent(parentId: string): void {


seems redundant

neSpecc · 2024-12-17T20:33:28Z

src/presentation/pages/Note.vue

+}
+
+/**
+ * Displayed parents


bad description

e11sy · 2024-12-17T20:45:33Z

src/application/services/useNote.ts

+   *
+   * Actual note by default
+   */
+  const noteParents = ref<Note[]>([]);


still actual

e11sy · 2024-12-17T20:49:05Z

src/presentation/pages/Note.vue

+const displayedParents = computed(() => {
+  if (noteParents.value.length > 3) {
+    const newNoteContent = { blocks: [] } as OutputData;
+
+    newNoteContent.blocks.push({ type: 'paraghraph',
+      data: { text: '...' } });
+
+    return [
+      noteParents.value[0],
+      { id: '',
+        content: newNoteContent },
+      noteParents.value[noteParents.value.length - 1],
+    ];
+  }
+
+  return noteParents.value;
+});


i would suggest moving all of this logic to different component of Notex (not codex-ui), then we will not overcomplicate Note page

i think this is discussable, @neSpecc what do you think about this?

e11sy · 2024-12-17T20:52:36Z

src/application/services/useNote.ts

+   * @returns An array of Note objects representing the formatted note parents.
+   * @throws {Error} If the note id is not defined.
+   */
+  function formatNoteParents(): Note[] {


this method would be used only in component, so we should move this logic to computable displayedParents no need to produce extra entities (such as presentationFormat)

dependentmadani added 2 commits November 7, 2024 13:46

feat: display the note parents structure

73f4b36

fix: lint and build errors

6ba36ac

e11sy reviewed Nov 7, 2024

View reviewed changes

neSpecc reviewed Nov 8, 2024

View reviewed changes

dependentmadani and others added 2 commits November 8, 2024 16:13

update: few modification of format function as well the docs

d9988fb

update (useNote): small modification in format function, and update i…

7baf8f1

…n useNoteComposableState interface

e11sy reviewed Nov 27, 2024

View reviewed changes

src/application/services/useNote.ts Outdated Show resolved Hide resolved

dependentmadani added 3 commits November 27, 2024 17:48

update: fix description of a doc

931460d

update: few changes on how to display the note structure, still need …

441586d

…work to be done

update (note parent strcuture): update the presentation form, logic a…

04e849f

…pplied, still need work to be done

neSpecc reviewed Dec 14, 2024

View reviewed changes

update: changes based on the reviews, make things work as it should

e78539c

neSpecc reviewed Dec 17, 2024

View reviewed changes

e11sy reviewed Dec 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: display the note parents structure #275

feat: display the note parents structure #275

dependentmadani commented Nov 7, 2024

e11sy Nov 7, 2024

e11sy Dec 17, 2024

e11sy Nov 7, 2024

dependentmadani Nov 8, 2024

neSpecc Nov 8, 2024

neSpecc Nov 8, 2024

neSpecc Dec 14, 2024 •

edited

Loading

neSpecc Dec 14, 2024

neSpecc Dec 14, 2024

neSpecc Dec 14, 2024

neSpecc Dec 14, 2024

neSpecc Dec 14, 2024

neSpecc Dec 17, 2024

neSpecc Dec 17, 2024

neSpecc Dec 17, 2024

neSpecc Dec 17, 2024

neSpecc Dec 17, 2024

neSpecc Dec 17, 2024

e11sy Dec 17, 2024

e11sy Dec 17, 2024

e11sy Dec 17, 2024

e11sy Dec 17, 2024

feat: display the note parents structure #275

Are you sure you want to change the base?

feat: display the note parents structure #275

Conversation

dependentmadani commented Nov 7, 2024

Problem:

Solution:

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

neSpecc Dec 14, 2024 • 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

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

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

neSpecc Dec 14, 2024 •

edited

Loading