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

Support creating templates only using tags #11

Merged
merged 4 commits into from
Jul 31, 2021
Merged

Conversation

nishantwrp
Copy link
Collaborator

@nishantwrp nishantwrp commented Jul 26, 2021

This PR does two things.

  • Allow creating templates using a tag.
  • Remove templates notebook functionality.

Fixes #10, Fixes #12.

@CalebJohn
Copy link
Collaborator

Is this PR still a draft? My templates aren't migrated in to the new system. Also, the popup dialogue says that there is a help option under Templates, but it doesn't seem to exist yet.

@nishantwrp
Copy link
Collaborator Author

nishantwrp commented Jul 27, 2021

My templates aren't migrated in to the new system.

I'll create another PR for this.

Also, the popup dialogue says that there is a help option under Templates, but it doesn't seem to exist yet.

That will also be done in a separate PR. But wrote this so, I don't have to change it in future.

@CalebJohn
Copy link
Collaborator

I haven't merged this yet so that we can wait for the conclusion of #12.

@nishantwrp nishantwrp changed the title Allow using tags and existing notebook Support creating templates only using tags Jul 31, 2021
@nishantwrp
Copy link
Collaborator Author

nishantwrp commented Jul 31, 2021

@CalebJohn please take a look. I'll add the feature to load previous templates in a separate PR. So, this can be merged.

@@ -21,29 +12,5 @@ export const doesFolderExist = async (folderId: string): Promise<boolean> => {
};

export const getAllNotesInFolder = async (folderId: string): Promise<Note[]> => {
let pageNum = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this function (and module) needed anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, they're not thanks for pointing that out. If this is the only review comment, you can merge this pr, I'll remove these functions in the next pr.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good to see that we're on the same page. I'm looking forward to the next PR.

@CalebJohn
Copy link
Collaborator

I just have the one comment, I don't think that function is needed anymore, but it's something you can address later, I'll merge this now. It looks good!

@CalebJohn CalebJohn merged commit 5c88aa1 into master Jul 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants