Skip to content

Conversation

@blimmer
Copy link
Contributor

@blimmer blimmer commented Sep 25, 2016

By default, it will still default to the "Powered by Active Admin <version>", but this allows folks to customize that message.

We wanted to append additional environmental information to better understand bug reports from our internal customers. To accomplish this today, I created app/admin/footer.rb, but it would be nice to set this as a configuration value.

In determining how to do this with my app, I found several resources (stack overflow, gists, blog posts) which indicated to me that this would be a useful feature for people.

varyonic
varyonic previously approved these changes Jan 15, 2017
Copy link
Contributor

@varyonic varyonic left a comment

Choose a reason for hiding this comment

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

My preference would be to enable use of a partial but I am fine with this.

Copy link
Member

@timoschilling timoschilling left a comment

Choose a reason for hiding this comment

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

I agree with @varyonic, a partial is a better way.

@Fivell
Copy link
Member

Fivell commented Jan 21, 2017

what if this already works?

ActiveAdmin.application.footer = proc {  render_to_string partial: 'footer' }

but I haven't check

@Fivell
Copy link
Member

Fivell commented Jan 24, 2017

@blimmer, ping

@blimmer
Copy link
Contributor Author

blimmer commented Jan 24, 2017

I haven't had time to look into a partial and am not sure my timeline on availability to look. If this solution isn't acceptable it might make sense to close this PR and open an issue to track that implementation.

@varyonic
Copy link
Contributor

I mentioned preferring a partial, but I do think this is acceptable.

@blimmer
Copy link
Contributor Author

blimmer commented Jan 24, 2017

Agreed - it seemed like the investigation / implementation of the partial was blocking from the requested changes indication on the PR

@timoschilling
Copy link
Member

I close this PR, it should be implemented as parial or proc

@varyonic
Copy link
Contributor

@timoschilling I'm disappointed. Next time I won't suggest a preference when I approve.

@blimmer
Copy link
Contributor Author

blimmer commented Jan 25, 2017

Agreed.

This implementation is better than what we have now (which is no ability to customize the footer at all), IMO.

@timoschilling timoschilling reopened this Jan 29, 2017
@timoschilling timoschilling dismissed stale reviews from varyonic and themself January 29, 2017 18:42

let us discuss this again

Copy link
Member

@Fivell Fivell left a comment

Choose a reason for hiding this comment

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

for me common scenario is put company copyrights to footer , that almost always in this format

Company name - start year - current year.

So I can't use and still need monkey-patch this because current year should be evaluated


## Footer Customization

By default, Active Admin displayed a "Powered by ActiveAdmin" message on every
Copy link
Contributor

@varyonic varyonic Jan 29, 2017

Choose a reason for hiding this comment

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

...Active Admin displays...

# == Footer
#
# By default, the footer shows the current Active Admin version. You can
# override the content of the fotter here.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/fotter/footer/

Copy link
Contributor

@varyonic varyonic left a comment

Choose a reason for hiding this comment

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

This PR supports using a proc.

By default, it will still default to the "Powered by Active Admin
<version>", but this allows folks to customize that message.
@varyonic varyonic force-pushed the feature/add-to-footer branch from 7c16ae6 to c30dd56 Compare January 29, 2017 20:44
@varyonic
Copy link
Contributor

Fixed a couple of typos, squashed and force-rebased.

Copy link
Member

@Fivell Fivell left a comment

Choose a reason for hiding this comment

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

👍

@Fivell
Copy link
Member

Fivell commented Jan 29, 2017

closes #4758

@varyonic varyonic merged commit 2bb5021 into activeadmin:master Jan 29, 2017
@timoschilling
Copy link
Member

Maybe some can make a PR to add the proc support in the docs and the initializer template

@@ -0,0 +1,28 @@
Feature: Site title
Copy link
Contributor

Choose a reason for hiding this comment

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

s/title/footer/

@varyonic
Copy link
Contributor

Agreed, though much of this was copied from Greg's original site_title where this is not documented either.

@gkop
Copy link

gkop commented Mar 16, 2017

This is a nice improvement, thanks Ben and friends!

@varyonic
Copy link
Contributor

Ironically this was one of the topics of an old post by Philippe.

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.

5 participants