Closed chosak closed 6 years ago
This is looking and working great for me.
I added one small style tweak in a153adc to get the correct hover and active states on the link in the notification text, but otherwise I think this design is fine for now. In a future iteration I'd like to revisit the design of this landing page message alongside figuring out how this message should display on other pages of a regulation, but I don't think that work should hold up this PR.
Content-wise, I think this message works well on landing pages both with and without new amendment notifications. I'm still waiting to hear back from other stakeholders on the final wording, though. If they come back with any changes, I'll push an update to this branch.
Code-wise, I got the update notification to show up by defining EREGS_REGULATION_UPDATES
in a local_settings.py
file in this repo, which is the extent of my Django knowledge.
@chosak, I pushed one more commit to add the final styling and language. Do we need a third-party to do a review of everything?
@niqjohnson I tried this locally and it works well for me. This is a really nice design for the combined messages!
If you want to merge, go ahead, otherwise I can. Either way, once this is merged I can tag a new release of regulations-site (or show you how to, if you're interested). Then we'll need to open a related PR on cfgov-refresh to bump the version there and also set the appropriate value for settings.EREGS_REGULATION_UPDATES
.
This commit adds support for a new Django setting,
EREGS_REGULATION_UPDATES
, that allows for placement of a message on certain regulations to let users know that an update in progress. This allows for a central place to toggle these updates instead of requiring changes on each reg's specific landing page.Implementation is done via a new
{% update_in_progress %}
template tag that can be passed a regulation part, that checks the hardcoded list in settings.This change also removes the hardcoded update message that currently appears on part 1026 (Reg Z) and so any deployments will need to add the setting like this to maintain current behavior:
Documentation has been updated in the
README.md
file and new unit tests have been added.I'm assigning this PR to @niqjohnson pending his revisions on the design of the message, please feel free to push any desired changes to this branch.