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

Avoid rendering the side content when there's no content #859

Merged

Conversation

Davidslv
Copy link
Contributor

@Davidslv Davidslv commented Dec 2, 2016

This commit aims to superseed alphagov/frontend#1069
We believe that is the component responsability to make sure the requirements are met
in order to do the the job, in this case, to render the content.

What this commit suggest is that we do our components with Design by Contract in mind,
what it means is that the component becomes responsible to dictate how it should be used.

Also there's a certain advantage of doing this at the component level,
this avoids extra work on the developer to chase all the rendering applications to make sure they should or not render the component.

So keeping it on the component side is not only beneficial code wise but also more efficient.

Trello: https://trello.com/c/5TCdhKVN/320-fix-the-hovering-blue-sidebar-on-pages-without-related-links

screen shot 2016-12-01 at 14 57 01

screen shot 2016-12-01 at 14 57 12

@Davidslv Davidslv force-pushed the fix-the-hovering-blue-sidebar-on-pages-without-related-links branch from 147b003 to 7899575 Compare December 2, 2016 14:35
@@ -8,7 +8,6 @@ def component_name
test "no error if no parameters passed in" do
assert_nothing_raised do
render_component({})
assert_select ".govuk-related-items"
Copy link
Contributor

Choose a reason for hiding this comment

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

Some components use this:
assert_empty render_component({})

I think that would be better now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah! Thanks, that makes much more sense!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fofr I've updated the PR, thank you.

This commit aims to superseed alphagov/frontend#1069
We believe that is the component responsability to make sure the requirements are met
in order to do the the job, in this case, to render the content.

What this commit suggest is that we do our components with [Design by Contract](https://en.wikipedia.org/wiki/Design_by_contract) in mind,
what it means is that the component becomes responsible to dictate how it should be used.

Also there's a certain advantage of doing this at the component level,
this avoids extra work on the developer to chase all the rendering applications to make sure they should or not render the component.

So keeping it on the component side is not only beneficial code wise but also more efficient.

Trello: https://trello.com/c/5TCdhKVN/320-fix-the-hovering-blue-sidebar-on-pages-without-related-links

![screen shot 2016-12-01 at 14 57 01](https://cloud.githubusercontent.com/assets/136777/20798375/82f36314-b7d6-11e6-92cc-0196f3eedb85.png)

![screen shot 2016-12-01 at 14 57 12](https://cloud.githubusercontent.com/assets/136777/20798382/86fe78c2-b7d6-11e6-89ec-3497e14e7168.png)
@Davidslv Davidslv force-pushed the fix-the-hovering-blue-sidebar-on-pages-without-related-links branch from 7899575 to 702fe78 Compare December 2, 2016 16:36
@fofr fofr merged commit c6cb0f4 into master Dec 2, 2016
@fofr fofr deleted the fix-the-hovering-blue-sidebar-on-pages-without-related-links branch December 2, 2016 17:34
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.

None yet

2 participants