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

Tag Insertion Dialog #267

Merged
merged 16 commits into from
Jan 15, 2023
Merged

Tag Insertion Dialog #267

merged 16 commits into from
Jan 15, 2023

Conversation

braxtonhall
Copy link
Owner

Copy link
Collaborator

@aymendirar aymendirar left a comment

Choose a reason for hiding this comment

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

epicnessity

Comment on lines 16 to 21
// {
// kind: "button",
// text: "Insert Tags",
// colour: UIColour.GREY,
// onClick: async () => resolve(insertTags(invalidTags).then(() => saveHandler({noCache: true}))),
// },
Copy link
Collaborator

Choose a reason for hiding this comment

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

i know we're working towards some tickets in this PR, but do we want this here?

import {UIColour} from "../../../../../common/ui/colour";
import {getSheetLink} from "../../../../../common/entities/spreadsheet";
import {GetTagsOptions} from "../types";
// import {insertTags} from "./insertTags";
Copy link
Collaborator

Choose a reason for hiding this comment

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

same thing

)
);

export {insertTags};
Copy link
Collaborator

Choose a reason for hiding this comment

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

mmmm delicious file

for (let node = trees.get(tag.toLowerCase()); node; node = node.parent) {
ancestry.push(node.tag);
for (let node: TagTree["parent"] = nodes.get(tag.toLowerCase()); node && "parent" in node; node = node.parent) {
// TODO maybe we should detect collisions here and then create a modal?
Copy link
Collaborator

Choose a reason for hiding this comment

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

how often does this code get run? and what would the user do with this information if we show them the modal?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think maybe the modal should happen at a later step.
For example, when adding ancestor tags.

Or maybe? Instead of a tree its a network, and we just go for it and add all ancestors on multiple paths?

@braxtonhall braxtonhall merged commit 4c83885 into main Jan 15, 2023
@braxtonhall braxtonhall deleted the insert-tags branch January 15, 2023 13:55
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.

trim tags in sheet
2 participants