-
Notifications
You must be signed in to change notification settings - Fork 6
[147] hierarchical labels #176
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
Changes from all commits
edc4ddd
d65233b
9035ca8
cd6353c
fec5017
c48c3c0
7241f59
57265b5
69d78af
f8c527f
9e8787e
b4eb7ca
6624ff9
bc3dd75
ef4954d
15995eb
491682d
e5bd5ed
118ae21
438d324
646acdf
cfff3e1
e7ffc31
efad22d
5258c05
d58b7e7
7a240e0
a6ee5bd
36d49ea
184fc94
c51989f
8c10280
7241239
d2313f8
fffa90b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,8 +1,73 @@ | ||||||
| import { LABEL_LOCAL, LABEL_REMOTE } from "../NotesProvider/consts/labels"; | ||||||
| import { type IDecoratedNote, NoteSource } from "../NotesProvider/types/DecoratedNote"; | ||||||
| import type { IStorageNote } from "../NotesProvider/types/StorageNote"; | ||||||
| import "./Label.css"; | ||||||
| import { useMemo } from "react"; | ||||||
|
|
||||||
| export type ILabel = { | ||||||
| label: string; | ||||||
| isActive: boolean; | ||||||
| parent: ILabel | null; | ||||||
| children: ILabel[]; | ||||||
| notes: IDecoratedNote[]; | ||||||
| }; | ||||||
|
|
||||||
| export function generateLabelTree(notes: IDecoratedNote[]): ILabel[] { | ||||||
| const local: ILabel = { | ||||||
| label: LABEL_LOCAL, | ||||||
| isActive: true, | ||||||
| parent: null, | ||||||
| children: [], | ||||||
| notes: [], | ||||||
| }; | ||||||
| const remote: ILabel = { | ||||||
| label: LABEL_REMOTE, | ||||||
| isActive: true, | ||||||
| parent: null, | ||||||
| children: [], | ||||||
| notes: [], | ||||||
| }; | ||||||
|
|
||||||
| function addToTree(labelPath: string[], note: IDecoratedNote, currentNode: ILabel) { | ||||||
| if (labelPath.length === 0) { | ||||||
| currentNode.notes.push(note); | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| const [head, ...rest] = labelPath; | ||||||
| let childNode = currentNode.children.find((child) => child.label === head); | ||||||
| if (!childNode) { | ||||||
| childNode = { | ||||||
| label: head, | ||||||
| isActive: true, | ||||||
| parent: currentNode, | ||||||
| children: [], | ||||||
| notes: [], | ||||||
| }; | ||||||
| currentNode.children.push(childNode); | ||||||
| } | ||||||
| addToTree(rest, note, childNode); | ||||||
| } | ||||||
|
|
||||||
| for (const note of notes) { | ||||||
| for (const label of note.original.labels) { | ||||||
| const parts = label.trim().split("/"); | ||||||
| const root = note.source === NoteSource.Local ? local : remote; | ||||||
| if (parts.length === 1) { | ||||||
| if (parts[0] === LABEL_LOCAL || parts[0] === LABEL_REMOTE) { | ||||||
| root.notes.push(note); | ||||||
| continue; | ||||||
| } | ||||||
| } | ||||||
| addToTree(parts, note, root); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| return [local, remote]; | ||||||
| } | ||||||
|
|
||||||
| export function Label({ label, prefix = "" }: { label: string; prefix?: string }) { | ||||||
| const backgroundColor = useMemo(() => labelToColor(label), [label]); | ||||||
| const backgroundColor = useMemo(() => labelToColor(label.split("/").pop() || ""), [label]); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Background color should depend on the whole "path" (i.e. remote label
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it makes labels in notes and labels on the tree displays differently (w/ different color)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, we should have full path in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. full path displayed on tree?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I mean is that we should always use full path for color and display the text-label depending on the context:
|
||||||
| return ( | ||||||
| <span style={{ backgroundColor }} className="label"> | ||||||
| {prefix} {label} | ||||||
|
|
@@ -41,3 +106,59 @@ function hslToHex(h: number, s: number, lightness: number) { | |||||
| }; | ||||||
| return `#${f(0)}${f(8)}${f(4)}`; | ||||||
| } | ||||||
|
|
||||||
| export function filterNotesByLabels( | ||||||
| labels: ILabel[], | ||||||
| { onlyInactive }: { onlyInactive: boolean } = { onlyInactive: true }, | ||||||
| ): IStorageNote[] { | ||||||
| return filterDecoratedNotesByLabels(labels, { hasAllLabels: true, onlyInactive: onlyInactive }).map( | ||||||
| (note) => note.original, | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| export function filterDecoratedNotesByLabels( | ||||||
| labels: ILabel[], | ||||||
| { hasAllLabels, onlyInactive }: { hasAllLabels: boolean; onlyInactive: boolean } = { | ||||||
| hasAllLabels: true, | ||||||
| onlyInactive: false, | ||||||
| }, | ||||||
| ): IDecoratedNote[] { | ||||||
| function traverseAndCollectNotes(labels: ILabel[], notes: Set<IDecoratedNote>, isActive = true) { | ||||||
| for (const label of labels) { | ||||||
| if (label.isActive === isActive) { | ||||||
| for (const note of label.notes) { | ||||||
| notes.add(note); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (label.children) { | ||||||
| traverseAndCollectNotes(label.children, notes, isActive); | ||||||
| } | ||||||
| } | ||||||
| return notes; | ||||||
| } | ||||||
|
|
||||||
| // return notes from inactive labels | ||||||
| if (onlyInactive) { | ||||||
| return Array.from(traverseAndCollectNotes(labels, new Set(), false)); | ||||||
| } | ||||||
|
|
||||||
| const activeNotes = traverseAndCollectNotes(labels, new Set()); | ||||||
| if (hasAllLabels) { | ||||||
| const inactiveNotes = traverseAndCollectNotes(labels, new Set(), false); | ||||||
| for (const note of inactiveNotes) { | ||||||
| activeNotes.delete(note); | ||||||
| } | ||||||
| } | ||||||
| return Array.from(activeNotes); | ||||||
| } | ||||||
|
|
||||||
| export function getFullLabelName(label: ILabel): string { | ||||||
| let parentLabel = label.parent; | ||||||
| let fullName = label.label; | ||||||
| while (parentLabel) { | ||||||
| fullName = `${parentLabel.label}/${fullName}`; | ||||||
| parentLabel = parentLabel.parent; | ||||||
| } | ||||||
| return fullName; | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,42 +1,54 @@ | ||||||
| import "./LabelsFilter.css"; | ||||||
| import { type MouseEventHandler, useCallback } from "react"; | ||||||
| import { Label } from "../Label/Label"; | ||||||
| import type { ILabel } from "../NotesProvider/hooks/useLabels"; | ||||||
| import { useState } from "react"; | ||||||
| import { type ILabel, Label, getFullLabelName } from "../Label/Label"; | ||||||
|
|
||||||
| export type LabelsFilterProps = { | ||||||
| labels: ILabel[]; | ||||||
| onToggleLabel: (label: string) => void; | ||||||
| onToggleLabel: (label: ILabel) => void; | ||||||
| }; | ||||||
|
|
||||||
| export function LabelsFilter({ labels, onToggleLabel }: LabelsFilterProps) { | ||||||
| export function LabelsFilterTree({ labels, onToggleLabel }: LabelsFilterProps) { | ||||||
| return ( | ||||||
| <div className="labels filter"> | ||||||
| <div className="label-tree-content"> | ||||||
| {labels.map((label) => ( | ||||||
| <LabelLink key={label.label} label={label} onToggleLabel={onToggleLabel} /> | ||||||
| <LabelNode key={label.label} label={label} onToggleLabel={onToggleLabel} /> | ||||||
| ))} | ||||||
| </div> | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| type LabelLinkProps = { | ||||||
| label: ILabel; | ||||||
| prefix?: string; | ||||||
| onToggleLabel: LabelsFilterProps["onToggleLabel"]; | ||||||
| }; | ||||||
|
|
||||||
| function LabelLink({ label, onToggleLabel }: LabelLinkProps) { | ||||||
| const selectLabel = useCallback<MouseEventHandler>( | ||||||
| (e) => { | ||||||
| e.preventDefault(); | ||||||
| onToggleLabel(label.label); | ||||||
| }, | ||||||
| [label, onToggleLabel], | ||||||
| ); | ||||||
|
|
||||||
| function LabelNode({ label, onToggleLabel }: LabelLinkProps) { | ||||||
| const [expanded, setExpanded] = useState(true); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| const hasChildren = Object.keys(label.children).length > 0; | ||||||
| const prefix = hasChildren ? (expanded ? "▼" : "▶") : label.isActive ? "⊙" : "∅"; | ||||||
| const clazz = `label-link ${label.isActive ? "active" : ""}`; | ||||||
| const ico = label.isActive ? "⊙" : "∅"; | ||||||
|
|
||||||
| return ( | ||||||
| <a href="#" className={clazz} onClick={selectLabel}> | ||||||
| <Label label={label.label} prefix={ico} /> | ||||||
| </a> | ||||||
| <div className="label-node"> | ||||||
| <div | ||||||
| className="label-node-header" | ||||||
| onClick={() => onToggleLabel(label)} | ||||||
| onKeyUp={(e) => e.key === "Enter" && setExpanded(!expanded)} | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
that shouldn't be needed. The group should be expanded when it's active and folded when inactive. |
||||||
| tabIndex={0} | ||||||
| role="button" | ||||||
| > | ||||||
| <div className={clazz}> | ||||||
| <Label key={getFullLabelName(label)} label={label.label} prefix={prefix} /> | ||||||
| </div> | ||||||
| </div> | ||||||
| {expanded && hasChildren && ( | ||||||
| <div className="label-node-content"> | ||||||
| {Object.values(label.children).map((child) => ( | ||||||
| <LabelNode key={child.label} label={child} onToggleLabel={onToggleLabel} /> | ||||||
| ))} | ||||||
| </div> | ||||||
| )} | ||||||
| </div> | ||||||
| ); | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is more correct way to handle that case. I wonder however where does this special casing come from? Do we currently have notes with explicit
localorremotelabel within them? I think it's totally fine if they were displayed aslocal/local- the user may remove that label if it was added there unnecessarily.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our other mechanism continuously adds the ‘local’ label, resulting in ‘local/local’ on each page refresh.