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

Add nav menu field to scf #44

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from

Conversation

haseebnawaz298
Copy link

  • this is a very common field that is used a lot, and it is not in scf.

@kraftbj kraftbj self-requested a review March 1, 2025 13:14
@kraftbj kraftbj added this to the 6.5.0 milestone Mar 3, 2025
Copy link
Collaborator

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

This works seems to work well. A couple of questions:

  1. Themes without nav menus (e.g. Twenty Twenty Four) will output a notice on the post editor that the theme doesn't support it. I'm curious if there's a better way of handling it (notice on the field group editor? Disable the Nav Menu field when using a theme that doesn't support it? Hide the field on the post editor if the theme doesn't support it, etc.
  2. Could you walk me through how you're using this? I'd just like to understand the user journey on it. Something like "this post is about Cats, so I want to display my Animals-specific navigation menu under the top nav" with more flexibility than, say, logic by category?

I mainly want to ensure I'm understanding the need properly.

There are a couple of version updates needed (this will go in 6.5) and tweaking the docs with a typo. [I'm happy to make those edits myself before merge, so no worries there].

With the tutorial, please feel free to be more verbose. The docs, overall, are really spartan but I hope to flesh them out so someone could follow the tutorial from start to finish to add a field, use a field via the post editor, and output the field.

@haseebnawaz298
Copy link
Author

haseebnawaz298 commented Mar 5, 2025

  1. we can go with showing a message in field group editor and hiding the field from post editor, then we donot need to show any notices.
  2. I have worked on many sites where this was needed we did use a plugin for it, but that plugin is no longer maintained.

some of the common use case that i normally use is.

  • A second menu that can be set as a global (Theme Options) and also be edited in a page( Page options), if page option is set to be empty global one is fetched.
  • it also gives you the ability to show different menus based on different conditions

@haseebnawaz298
Copy link
Author

I would also love to help with the docs, just need a direction on how deep we want the docs to be. eg: do we need to give code examples etc, or just basic instructions?

@kraftbj
Copy link
Collaborator

kraftbj commented Mar 5, 2025

  1. we can go with showing a message in field group editor and hiding the field from post editor, then we donot need to show any notices.

Awesome; are you good with making those changes on this branch before merging? (note that I did commit the suggestions I made in the initial review so please pull before new commits)

I would also love to help with the docs, just need a direction on how deep we want the docs to be. eg: do we need to give code examples etc, or just basic instructions?

I'd say basic code examples. I'm thinking of two types of users using the docs:

  1. The experienced person looking for details. We don't need to do too much beyond good inline documentation of the args, describing the options, etc.
  2. Someone new to WordPress/web development who need something more like training wheels.

For the latter person, if they said "I want to add a new field to my site" and stumbled upon SCF, will the tutorial provide them enough information to be able to add the field and output it? Until we have better integration with blocks, that's going to almost always be with small php code.

So, for the nav menu, I would say it could be something like "To output this field, add this code to the theme template, functions.php or elsewhere as needed for implementation ..... get_field(.....)......"

Then with a small tutorial showing "As an example, using a child theme of the Twenty Twelve default theme, if your nav menu field is "special-menu", add [code] to the such and such template.php"

Does that make sense? Obviously few people would need the specific tutorial exactly as written, but enough where someone could get it work in that generic place and be enough to hopefully connect some dots for them.

Additionally, I am okay with merging this (with the notice update) without the docs being 100% and accepting a follow-up PR for it.

Thank you!

@haseebnawaz298
Copy link
Author

i will work on this hopefully today or tomorrow, will get back to you soon

@haseebnawaz298
Copy link
Author

This is how the notices show in the group field editor now,

field is hidden if there is no support in theme.

image
image

@haseebnawaz298
Copy link
Author

also updated the docs to be a bit better

- hide field in post editor if there is no support
- show notice in field group editor
Copy link
Collaborator

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

I haven't actually tested it yet, but a couple of comments from reviewing the code.

@haseebnawaz298 haseebnawaz298 requested a review from kraftbj March 18, 2025 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants