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.

Sign up for GitHub

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

Jump to bottom

Add custom look #2803

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add custom look #2803

wants to merge 2 commits into from

Conversation

goosys
Copy link
Contributor

@goosys goosys commented Mar 11, 2025 •
edited
Loading

Introduces a custom look implementation based on the discussion in #2291.

Changes:

  • Added look option to FieldBase
    • Added RSpec tests for FieldBase
    • Updated RSpec tests for each Field
  • Added look option to generators
    • Field generator
    • Custom Field generator
  • Updated documentation
    • Custom Dashboard
    • Field generator
    • Custom Field generator
  • Added example for changing BelongsTo look in demo app
    • product: Field::BelongsTo.with_options(look: :product_card)
    • The Product card now displays the product image and name side by side.

Please review and let me know if any changes are needed! 🚀

Usage:

class PageDashboard < Administrate::BaseDashboard
  ATTRIBUTE_TYPES = {
    product: Field::BelongsTo.with_options(look: :product_card)
  }
end
class LineItemDashboard < Administrate::BaseDashboard
  ATTRIBUTE_TYPES = {
    product: Field::BelongsTo.with_options(look: :product_card)
  }
end

image

image

@goosys goosys mentioned this pull request Mar 11, 2025
Open
@goosys
Copy link
Contributor Author

goosys commented Mar 14, 2025

I added a sample since #2354 might also be easily resolved with looks. f280d50

image

@goosys
Copy link
Contributor Author

goosys commented Mar 20, 2025

@pablobm @nickcharlton

I've been playing around with this for the past few days, and it's incredibly fun! 🎉
I feel like this significantly expands the possibilities for customizing Administrate.
Some simple third-party custom field plugins could already be replaced with the looks feature.
Would you like to give it a try?

Also, could you update the beta tag to beta4 once looks is merged?
With virtual field, partial prefixes, and looks as the foundation, plugin development should become easier than before. Having an updated tag would be really helpful.

@nickcharlton
Copy link
Member

Yay! Thanks for doing this.

I think it's a big enough conceptual change it should probably go in after v1, if not just so that we can highlight the new functionality and it not get lost in other changes. But I'm still thinking this through!

@nickcharlton nickcharlton added views-and-styles how administrate looks and is interacted with feature new functionality that’s not yet implemented dashboards how administrate presents fields and displays data fields new fields, displaying and editing data install initial setup, first-run experience, generators generators labels Mar 20, 2025
@nickcharlton nickcharlton added this to the Post v1 milestone Mar 20, 2025
@goosys
Copy link
Contributor Author

goosys commented Mar 20, 2025

@nickcharlton

Thanks! That makes sense.

That said, since looks doesn’t break compatibility, I believe there’s no need to delay merging it. It’s stable as is, and if anything is missing, I’d be happy to help improve it further.

My application really depends on both looks and contextualize. Would it be possible to merge it now? 😭

@nickcharlton
Copy link
Member

The code change is small, but it's a big (positive) step in functionality which makes a nice highlight for a next release. I'm just trying to see if I can draw a line and get v1 done, as I've dragged my feet on it much longer than I was hoping I'd do.

I appreciate this makes things annoying for you and your application though 😭

@goosys
Copy link
Contributor Author

goosys commented Mar 20, 2025

Thank you! I really appreciate it. I’m looking forward to the completion of v1. 😊

pablobm
pablobm reviewed Mar 21, 2025
View reviewed changes
Copy link
Collaborator

@pablobm pablobm left a comment

Choose a reason for hiding this comment

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

Looking great! 🙂 Let some minor comments and after that I think this is a good first feature for the post-v1 release.

lib/generators/administrate/field/field_generator.rb
class_option(
:look,
type: :string,
desc: "Specify the look for the field",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps this is a bit clearer?

Suggested change
desc: "Specify the look for the field",
desc: "Generate templates for a \"look\" (alternative representation of the field)

lib/generators/administrate/views/field_generator.rb
class_option(
:look,
type: :string,
desc: "Specify the look for the field",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above:

Suggested change
desc: "Specify the look for the field",
desc: "Generate templates for a \"look\" (alternative representation of the field)

But that's assuming this is relevant here, as per my question on whether this generator should be used on its own.

docs/customizing_attribute_partials.md
If you want to generate a different look, run the following:

```bash
rails generate administrate:views:field number --look custom
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this actually work? I've been trying today and it didn't work, and only now I'm realising perhaps this generator isn't supposed to run on its own.

docs/adding_custom_field_types.md

This will generate three files:

- `app/fields/gravatar_field.rb`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- `app/fields/gravatar_field.rb`
- `app/fields/gravatar_field.rb` (assuming it doesn't exist already)

lib/administrate/field/base.rb
def self.local_partial_prefixes
["fields/#{field_type}"]
def self.local_partial_prefixes(look: :default)
fallback = ["fields/#{field_type}/looks/default", "fields/#{field_type}"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to allow for default? I think it might get confusing and affect maintenance in the long run. There's already the normal option (no look), so not sure we need this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think having both would cause significant confusion, but which approach do you prefer?

I felt that plugin developers might prefer having everything neatly organized within looks dir, so I designed it this way to allow both options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pablobm pablobm pablobm left review comments

Assignees
No one assigned
Labels
dashboards how administrate presents fields and displays data feature new functionality that’s not yet implemented fields new fields, displaying and editing data generators install initial setup, first-run experience, generators views-and-styles how administrate looks and is interacted with
Projects
None yet
Milestone
Post v1
Development

Successfully merging this pull request may close these issues.

Multiple "looks" per Field
3 participants
@goosys @nickcharlton @pablobm