-
-
Notifications
You must be signed in to change notification settings - Fork 405
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
Highlighted content banner #2572
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2572 +/- ##
==========================================
+ Coverage 98.82% 98.82% +<.01%
==========================================
Files 1435 1435
Lines 33522 33592 +70
==========================================
+ Hits 33128 33198 +70
Misses 394 394 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, you're adding a decidim-system/decidim-system-0.9.0.pre.gem
file that shouldn't be there
decidim-admin/config/locales/ca.yml
Outdated
highlighted_content_banner_action_title: Títol del botó d'acció | ||
highlighted_content_banner_action_subtitle: Subtítol del botó d'acció | ||
highlighted_content_banner_action_url: URL del botó d'acció | ||
highlighted_content_banner_image: Imatge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, only modify the en.yml
locales files
I don't understand this PR. Is this coming from the @decidim/lot-px team? Apart from that, can you add some help text explaining where will this be shown? |
Also, as per #2516, you'll need to move all |
This looks quite arbitrary; it might not be useful to everyone. I think changes like this should be done at the end application (by overriding partials or using deface to extend an existing view like here). Also, there's lots of other unrelated files in the PR, looks like a bad merge! |
Last rebase has been a pain in the ass. I will fix the merge. With regards to the intent of this feature, better discuss it with @xabier |
Adds hightlighted content banner feature
4d49d3e
to
5a6e4d7
Compare
Since we have an alternative implementation based on ViewHooks I decided to close this PR. |
Adds a banner that highlights content on home page as well as the
required administration page.
🎩 What? Why?
Adds a banner in the home page that can be configured to highlight content. It is a requirement from decidim-consultations.
Public home looks like this:
Administration interface looks like this:
📌 Related Issues