-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Model display name fixes #319
Model display name fixes #319
Conversation
When assets have been copied with rake admin:copy_assets, then rails_admin should NOT insert another ActionDispatch::Static middleware, because it eclipses Rails's native middleware and assets are loaded from rails_admin's public folder instead of the application's public folder. Fixed by checking if any of the rails_admin subdirectories exist in any of the static asset classes (images, javascripts, stylesheets). Also, if the middleware is inserted, it should be done AFTER, not IN FRONT of Rails's own static asset middleware, to permit precedence to the application's public folder over the gem's.
…g, but didn't check for the files to be present, which is cool.
Model name in table is now using label override if present.
@page_name should use overridden model name. Same for breadcrumbs. Spec coverage added.
where @model_config.list.label was called, but @model_config.navigation.label should have been called.
Hi Wolfram and thank you for the fix! Your approach is on the right track, but I think it should be taken bit further. The old code allowed to define label per section (navigation, update, create, list), but that was indeed an overkill and wasn't even implemented in the dashboard. I think simplifying the label definition to a single point of entry is definitely a good idea, but it should not be contained in the navigation section, but on a higher level. Therefore I'd propose we'd move In configuration one would use:
And in code:
Do you find this proposal acceptable and change the pull request accordingly? PS. I'm also thinking we should do same kind of move with |
Hi Petteri, Thanks for your reply. To be honest, I hadn't dove deep enough into the architecture to fully understand the bigger picture. Conceptually what you're proposing makes sense, but I'll have to get my head wrapped around the configuration layer in more detail than I had previously. I'll give it a try. Best, |
Can this pull request be close since I've merged #322? |
Yes this can be closed. It was submitted from my co-worker's account at Papercheck. I'll do it. Sorry I forgot. I appreciate your diligence. Best, |
The model label is now configurable at the model level only, and is no longer configurable at the section level (list, navigate, update,...), as this was considered overkill. This refactoring was in part motivated by issue #319 which reported that display of labels was very inconsistent across various screens, and the label configuration, if given, was not consistently effective. The Labelable module was removed, and the methods model into config/model.rb All references to label across the code have been updated to use the model configuration. Specs updated and passing. Readme also updated accordingly.
When the model name has been modified with:
that name isn't used when it should be used for the dashboard table, the breadcrumbs, page name, history entries and delete messages.
This fixes issue: https://github.com/sferik/rails_admin/issues/#issue/318