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

Suggested councillor council name #1185

Conversation

hisayohorie
Copy link
Contributor

@hisayohorie hisayohorie commented Jun 22, 2017

This will display the name of council the suggested councillors belong.

screen shot 2017-06-21 at 11 50 27 pm

Fixes #1181 .

The test for Council Name is nested in the feature_flag_is_on test, so
that the feature is visible in the test environment.
Copy link
Member

@henare henare left a comment

Choose a reason for hiding this comment

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

Great, simple implementation that does the basics first. I think it could be worth improving the text to be more helpful and descriptive.

@hisayohorie would you like to do that or would you prefer we just merge this so you can move on to something else first?

@@ -1,3 +1,6 @@
%p
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a header instead of a paragraph?

@@ -1,3 +1,6 @@
%p
Council Name: #{@authority.full_name}
Copy link
Member

Choose a reason for hiding this comment

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

Since this line is so short it could go on the same line as the tag, i.e.:

%p Council Name: #{@authority.full_name}

to make it more readable and clear. still need some styling.
To separate the test exercise and test expectation.
Copy link
Member

@henare henare left a comment

Choose a reason for hiding this comment

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

@hisayohorie and I left the h1 text with the default styling because that will be done in #1180. We also decided to leave adding an introductory paragraph until we add other helpful text in #1182.

@henare henare merged commit 6c15cb0 into openaustralia:person_contributes_councillor_info Jun 22, 2017
@hisayohorie
Copy link
Contributor Author

I think the styling and wording are addressed in different issues #1179 #1180

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.

2 participants