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

Show when entities are closed on their homepage #1727

Merged
merged 2 commits into from
Jun 5, 2019

Conversation

inglesp
Copy link
Contributor

@inglesp inglesp commented Jun 4, 2019

Practice:

image

Regional team:

image

@inglesp inglesp requested review from evansd and sebbacon June 4, 2019 14:30
@inglesp
Copy link
Contributor Author

inglesp commented Jun 4, 2019

This fixes part of #889.

@HelenCEBM
Copy link
Contributor

Can we make the text stand out a bit more? e.g. "CLOSED: 16 May 2018" or use red maybe?

Copy link
Collaborator

@evansd evansd left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think it would be fine to wrap it in a <span class="text-danger"> if @HelenCEBM thinks that would be helpful, but from my own point of view it's OK as it is.

@inglesp
Copy link
Contributor Author

inglesp commented Jun 4, 2019

Here it is wrapped in a <span class="text-danger">.

image

What do you think @HelenCEBM?

@HelenCEBM
Copy link
Contributor

Yes great thanks!

Copy link
Contributor

@sebbacon sebbacon left a comment

Choose a reason for hiding this comment

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

LGTM. In case you've not noticed, you appear not to have pushed the red styling.

@inglesp
Copy link
Contributor Author

inglesp commented Jun 5, 2019

In case you've not noticed, you appear not to have pushed the red styling.

Pushed now.

@evansd
Copy link
Collaborator

evansd commented Jun 5, 2019

👍 Looks good to me

@inglesp inglesp merged commit bdb40a7 into master Jun 5, 2019
@inglesp inglesp deleted the show-entity-status-on-homepage branch June 5, 2019 15:10
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.

4 participants