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

Feat: content in sub folders #2897

Merged
merged 23 commits into from
Nov 28, 2019
Merged

Feat: content in sub folders #2897

merged 23 commits into from
Nov 28, 2019

Conversation

erezrokah
Copy link
Contributor

@erezrokah erezrokah commented Nov 19, 2019

Fixes: #513

Still work in progress, though I tested with github/gitlab/bitbucket.

Enable it by adding content_in_sub_folders: true to your folder collection (still need to write the documentation).
By enabling the flag slashes will not be sanitized and will be used for directory structure.
e.g doing:

The slug option remains the same.
You can add a path template for a collection similar to the slug generation template:

folder: 'posts'
slug: '{{month}}-{{day}}-{{slug}}'
path: '{{year}}/{{slug}}'

Will result in the file saved in posts/2019/11-19-title.md for example.

The slug key in the path template is the post format value of the slug.

TODO:

  • Think about filtering items when reading them from the repository
  • Think if I really need all the encoding/decoding
  • Fix GraphQL query to get nested trees
  • Use path config instead of content_in_sub_folders and keep slug behaviour
  • UI improvements
  • Test with open authoring
  • Only sanitize template variables. BREAKING CHANGE - fixes Filename sanitation should only apply to variable parts #1254

Things to consider:

  • POSSIBLE BREAKING CHANGE - folder collections entries are now retrieved up to a depth of 10 for all the backends. If someone relied on the CMS retrieving only up to a depth of 1, they might experience unwanted behavior.
  • Filtering entities on extraction - feels like a separate issue that can be done once we have more use cases.

Further improvements (I think they should be done in another PR):

internally we should not use slug in our redux store (and everywhere else for that matter). We should use the entry full path and just URI encode it to make sure it doesn't break the routing.
We should also not reference the term slug in the backends (specifically the GitHub one).

@erezrokah erezrokah changed the title Feat: content in sub folders [WIP] Feat: content in sub folders Nov 19, 2019
@tomrutgers
Copy link
Contributor

Question: do we need the content_in_sub_folders property? The slashes in the slug already imply that right?

@erezrokah
Copy link
Contributor Author

Question: do we need the content_in_sub_folders property? The slashes in the slug already imply that right?

That would make it a breaking change I think

@chrfritsch
Copy link
Contributor

Would a site structure like the following also be handled by this?

  • index.md
  • contact.md
  • teams
    • team1.md
    • team2.md
  • aboutus.md

@erezrokah
Copy link
Contributor Author

Would a site structure like the following also be handled by this?

  • index.md

  • contact.md

  • teams

    • team1.md
    • team2.md
  • aboutus.md

I think you'll need to set up two collections, one for the teams nested directory and one for the parent, then define a filter for each collection (We're still thinking on how to do the filtering as the current mechanism is very limited).

@chrfritsch
Copy link
Contributor

Uhh, that's so not nice TBH. Creating a site hierarchy is part of the editors' work (not developers) and should not depend on changing the cms config.

@erezrokah
Copy link
Contributor Author

erezrokah commented Nov 20, 2019

Uhh, that's so not nice TBH. Creating a site hierarchy is part of the editors' work (not developers) and should not depend on changing the cms config.

Actually this might be possible with a single collection if you use a field with the slug template.
e.g. {{fields.slug}} then have the editor update that field in the UI.

Another improvement will be to preview the destination path in the UI (like suggested in the issue thread).

@chrfritsch
Copy link
Contributor

Uhh, that's so not nice TBH. Creating a site hierarchy is part of the editors' work (not developers) and should not depend on changing the cms config.

Actually this might be possible with a single collection if you use a field with the slug template.
e.g. {{fields.slug}} then have the editor update that field in the UI.

Another improvement will be to preview the destination path in the UI (like suggested in the issue thread).

Ok, I will give it a try. Thank you. :)

@barthc
Copy link
Contributor

barthc commented Nov 24, 2019

@erezrokah path preview:
Peek 2019-11-24 23-57

And of course would need some UI improvements.

@erezrokah erezrokah force-pushed the feat/content_in_sub_folders branch 2 times, most recently from 984a745 to d4e71ee Compare November 26, 2019 10:20
@@ -55,6 +56,7 @@ export default class ControlPane extends React.Component {

return (
<ControlPaneContainer>
{collection.has('path') && <PathPreview collection={collection} entry={entry} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice if the path preview can come immediately before the identifier field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea I'll update it

try {
// slugFormatter throws in case an identifier is missing from the entry
// we can safely ignore this error as this is just a preview path value
return slugFormatter(collection, entry.get('data'), config.get('slug'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to add the file extension.

@erezrokah erezrokah changed the title [WIP] Feat: content in sub folders Feat: content in sub folders Nov 26, 2019
@@ -186,6 +186,22 @@ export default class GraphQLAPI extends API {
}
}

addFiles(allFiles, entries, path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

array reduce would be more cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking something like so:

function addFiles(entries, path) {
  return entries.reduce((acc, item) => {
    if (item.type === 'tree') {
      return [...acc, ...addFiles(item.object.entries, `${path}/${item.name}`)];
    }

    return [
      ...acc,
      {
        ...item,
        path: `${path}/${item.name}`,
        size: item.blob && item.blob.size
      }
    ];
  }, []);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool

@erezrokah erezrokah force-pushed the feat/content_in_sub_folders branch from 36eceaf to 9f0dd4e Compare November 27, 2019 09:14
@@ -92,17 +93,47 @@ export const statues = gql`
${fragments.object}
`;

const buildFilesQuery = (depth = 10) => {
const PLACE_HOLDER = 'PLACE_HOLDER';
Copy link
Contributor

Choose a reason for hiding this comment

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

Good thinking with the PLACE_HOLDER 👍

@erezrokah
Copy link
Contributor Author

PathPreview

@erezrokah erezrokah force-pushed the feat/content_in_sub_folders branch from 451661f to 10ad13d Compare November 27, 2019 10:30
@erquhart
Copy link
Contributor

Two issues with displaying the path:

  • most nontechnical users won’t need the info and may be confused by it
  • multiple fields could be involved in path formation

What’s more likely to be sought by an editor is the resulting URL path. We could handle this in a separate issue/PR and make some product decisions there.

What do you all think, agree/disagree?

Sent with GitHawk

@erezrokah
Copy link
Contributor Author

erezrokah commented Nov 27, 2019

Two issues with displaying the path:

  • most nontechnical users won’t need the info and may be confused by it
  • multiple fields could be involved in path formation

What’s more likely to be sought by an editor is the resulting URL path. We could handle this in a separate issue/PR and make some product decisions there.

What do you all think, agree/disagree?

Sent with GitHawk

I agree the preview is very raw atm. Having the resulting URL path sounds much better

@erezrokah
Copy link
Contributor Author

Removed the path preview completely for now

Copy link
Contributor

@erquhart erquhart left a comment

Choose a reason for hiding this comment

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

Killer work, Erez 💪 - let's test for the possible breaking change you mentioned, but I'm good with merging and publishing to beta as is.

@erquhart erquhart merged commit afcfe5b into master Nov 28, 2019
@erquhart erquhart deleted the feat/content_in_sub_folders branch November 28, 2019 03:39
@erquhart
Copy link
Contributor

erquhart commented Nov 28, 2019

Actually, second thought, I'm going to hold off on releasing since we're going into holiday, and many have come to depend on our long lived beta channel not breaking. If folks here really want to play with it over the next couple of days, make some noise.

@chrfritsch
Copy link
Contributor

I think many people, including me, are waiting for this feature. So please publish it 😊💪🏻🔥🚀

@flyfoxuk
Copy link

Yes please, would like to test it

@earthboundkid
Copy link

Can this be turned off? It sort of broke my content organization.

nathankitchen pushed a commit to nathankitchen/netlify-cms-backend-azure that referenced this pull request Feb 24, 2020
@hulloanson
Copy link

hulloanson commented Dec 21, 2020

Edit: rephrased

I have a repo structured like this:

  • /
    • blog 1
      • index.md
      • audio.mp3
      • image.jpg
    • blog 2
      • index.md
      • another_audio.mp3
      • more_audio.mp3
      • even_more_audio.mp3

etc...

and the following collection configuration (irrelevant fields not listed):

- folder: "/"
  slug: "{{slug}}"
  path: "{{slug}}/index"
  media_folder: ""
  public_folder: ""

Is it the intended way to use this feature? If not, how should I author on netlify cms with this structure?

I tried using the feature without success of course. Description of what happened below if you're interested.

------- Here is the experiment results ----------

Expected behavior:

Lists entries like "blog 1", "blog 2", etc.

Actual behavior:

Produced something like this:

image

Before going any further though, I do understand the empty entries are media files. I probably need to filter them.

The problem is: every time I scrolled to the bottom, all entries with text updated to the same name it happened to load from the last request. Such entries all have the link `https://example.com/#/collections/bites/entries/index

My uneducated guess was the "/index" I wrote in path / the "index.md" in each sub-folder messed with redux store keys but I have not studied the implementation... yep. that's all I've tried. just sharing. my concern is still on how I should author with the file structure I proposed.

Thank you!

@erezrokah
Copy link
Contributor Author

Hi @huguestennier, I would suggest moving your content into a dedicated directory, e.g. content and then use:

- folder: "content"
  slug: "{{slug}}"
  path: "{{slug}}/index"
  media_folder: ""
  public_folder: ""

If that doesn't work I suggest opening a new issue for it.

@hulloanson
Copy link

hulloanson commented Dec 22, 2020

Thank you @erezrokah ! It works if I put contents in a subfolder :) I don't even have to filter media files!

Amazing work!

vladdu pushed a commit to vladdu/netlify-cms that referenced this pull request Jan 26, 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
Development

Successfully merging this pull request may close these issues.

Filename sanitation should only apply to variable parts content in subfolders…
8 participants