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

fix: add folder pages for empty, intermediate folders #1295

Open
wants to merge 5 commits into
base: v4
Choose a base branch
from

Conversation

tha00
Copy link

@tha00 tha00 commented Jul 28, 2024

This PR adds folder pages for intermediate folders such as in

content/foo/bar/my-note.md

Before, only a folder page for bar was created, resulting in a 404 when clicking the foo link in Home > foo > bar, see issue #1084.

With the change, there exists also a (albeit empty) page for foo. A potential follow-up would be to show not only notes, but also subfolders on folder pages.

@tha00 tha00 marked this pull request as ready for review July 28, 2024 07:16
@aarnphm
Copy link
Collaborator

aarnphm commented Jul 30, 2024

Do you know what the default behaviour of Obsidian Vault is for this case?

Comment on lines 78 to 83
allFiles.flatMap((data) => {
const slug = data.slug
const folderName = path.dirname(slug ?? "") as SimpleSlug
if (slug && folderName !== "." && folderName !== "tags") {
return [folderName]
}
return []
return slug
? _getFolders(slug).filter((folderName) => folderName !== "." && folderName !== "tags")
: []
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can rewrite L78-82:

        allFiles.flatMap((data) =>
          data.slug
            ? getFolders(data.slug).filter(
                (folderName) => folderName !== "." && folderName !== "tags",
              )
            : [],
        ),

Copy link
Author

Choose a reason for hiding this comment

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

Thanks! I adapted my change.

@tha00
Copy link
Author

tha00 commented Aug 8, 2024

Do you know what the default behaviour of Obsidian Vault is for this case?

Sorry, could you please explain in more detail? I've observed the 404s when using quartz with Obsidian.

If you mean Obsidian Publish, then I don't know, I haven't used it.

@aarnphm
Copy link
Collaborator

aarnphm commented Aug 9, 2024

Yes, sorry I meant Obsidian Publish.

@jackyzha0
Copy link
Owner

I think we should do

  1. Generate intermediate folder pages (like you do here)
  2. Also account for these pages in the folder listing itself so the folder listing pages contain sublists

@tha00
Copy link
Author

tha00 commented Aug 15, 2024

I added a list of (direct) subfolders to the folder page. Do you think this is going in the right direction?

What's still missing is:

  • formatting/style
  • internationalization of the "x subfolders" text

subfolders

@aarnphm
Copy link
Collaborator

aarnphm commented Aug 15, 2024

I added a list of (direct) subfolders to the folder page. Do you think this is going in the right direction?

What's still missing is:

  • formatting/style
  • internationalization of the "x subfolders" text

subfolders

no need to have the subfolder, just include as item like before

@tha00
Copy link
Author

tha00 commented Aug 16, 2024

no need to have the subfolder, just include as item like before

I added the folders as regular items to the page list. Not sure, if they should have a date (e.g. latest date of their contents) or not?

subfolders2

@aarnphm
Copy link
Collaborator

aarnphm commented Aug 16, 2024

hmm, you can use the date from filesystem?

edit: my guess is dates with latest file content is fine.

@tha00
Copy link
Author

tha00 commented Aug 16, 2024

Added the latest dates of all files underneath the subfolder to the subfolder itself (reusing byDateAndAlphbetical).

@tha00
Copy link
Author

tha00 commented Aug 29, 2024

With the latest dates from their contents, the folder pages look like this now:

subfolders3

Anything I could do to improve the PR?

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.

3 participants