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 dashboard configuration to (not) be displayed in navigation #1418

Conversation

robinboening
Copy link

  • Introduces general configuration for Dashboards.
  • By default every Dashboard is configured to be displayed in the navigation.

How to remove a Dashboard from navigation:

class LineItemDashboard < Administrate::BaseDashboard
  configure do |config|
    config.navigation = false
  end
end

expect(navigation).to have_link("Blog/Posts")
end

it "does not display links to resources configured to not be rendered in navigation" do
Copy link

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [89/80]

lib/administrate/base_dashboard.rb Outdated Show resolved Hide resolved
lib/administrate/base_dashboard.rb Outdated Show resolved Hide resolved
lib/administrate/base_dashboard.rb Show resolved Hide resolved
@robinboening
Copy link
Author

Fixes #1417

@robinboening
Copy link
Author

FYI: rebased with current master in order to not run into issues with bundler 2 on CI (#1420)

@robinboening robinboening force-pushed the feature/dashboard_configuration branch 3 times, most recently from 540d098 to 314bd92 Compare September 7, 2019 13:12
+ Introduces configuration for Dashboards. (Easily extendedable for further configuration options)
+ By default every Dashboard is configured to be displayed in the navigation.

How to remove a Dashboard from navigation:

```
class LineItemDashboard < Administrate::BaseDashboard
  configure do |config|
    config.navigation = false
  end
end
```
but still be nested in another resource. In this case you keep the route but configure the dashboard to not be displayed in the navigation.

```ruby
class LineItemDashboard < Administrate::BaseDashboard

Choose a reason for hiding this comment

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

What would you think of making this name more abstract or generic? Something like RelationalRecordsDashboard or AssociationsDashboard ? I found myself wondering if LineItem had any significance, and am curious to know if anyone else paused at that.

Thanks for working towards an enhancement to administrate!

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.

Thank you for this PR @robinboening!

I've been looking at it, and I'm not sure this change aligns well with Administrate's spirit of doing the right thing with minimal configuration. So far we don't have any "configuration" APIs like the one you propose.

Thinking about the problem at hand, an important consideration is that resources :line_items shouldn't be removed. Instead it should be told to not have an :index route. That would be more "Rails-y":

resources :line_items, except: [:index]

Of course Administrate doesn't deal well with this, as it still renders the navigation. So the next question is: why does it render it if there's no index action?

The navigation is based off Administrate::Namespace#resources. I'm thinking that something here might be what needs changing: either the #resources method, or the fact that #resources is used as source of navigation items in the first place.

Ideally I think we should have something like valid_action?(:index, resource_class) be the question asked for whether each resource class should be in the navigation. So perhaps this should be used in that loop for resources in the navigation template?

This way Administrate should be doing the right thing without having to add any special configuration! What do you think? If this sounds good (and I'm not missing anything important!), I'd love a PR 😄

@pablobm
Copy link
Collaborator

pablobm commented Jan 18, 2020

This has now been implemented by #1524

@pablobm pablobm closed this Jan 18, 2020
@robinboening robinboening deleted the feature/dashboard_configuration branch May 9, 2020 20:06
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.

None yet

3 participants