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

Render custom organisation logos when provided #246

Merged
merged 2 commits into from Feb 17, 2017
Merged

Render custom organisation logos when provided #246

merged 2 commits into from Feb 17, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Feb 8, 2017

Goes with:

Part of https://trello.com/c/HtSGDoRb/594-add-organisation-logos-that-are-images-to-content-item-2

  • Pass custom organisation images to component
  • Restrict width of component to 25% to match HTML publication
  • Include image only when provided
  • Don't show custom logos on HTML pubs with many orgs

HTML pubs

screen shot 2017-02-08 at 15 50 52

Corp info pages

screen shot 2017-02-08 at 15 51 08

* Restrict width of component to 25% to match HTML publication
* Include image only when provided
@@ -31,4 +31,12 @@ def last_changed
def organisations
content_item["links"]["organisations"].sort_by { |o| o["title"] }
end

def organisation_logo(organisation)
super(organisation).tap do |logo|

This comment has been minimized.

@gpeng

gpeng Feb 10, 2017
Contributor

You don't need to pass the organisation argument here. It happens automagically.

This comment has been minimized.

@fofr

fofr Feb 16, 2017
Author Contributor

Fixed and rebased

example_logo = schema_item("updated")["links"]["organisations"][0]["details"]["logo"]["image"].symbolize_keys
presented_logo = presented.organisation_logo(organisation)[:organisation][:image]

refute presented.organisations.count > 1

This comment has been minimized.

@gpeng

gpeng Feb 10, 2017
Contributor

This hurts my head a bit. assert count < 2 or assert_equal 1, presented.organisations.count?

This comment has been minimized.

@fofr

fofr Feb 16, 2017
Author Contributor

Fixed and rebased

* Custom organisation logos have varying heights and give undue
prominence to those logos with a custom image, even if they are a
secondary org
* Keep the previous default which was to show a text only link for
organisations with a custom logo
@gpeng
gpeng approved these changes Feb 16, 2017
@fofr fofr changed the title [Do not merge] Render custom organisation logos when provided Render custom organisation logos when provided Feb 17, 2017
@fofr fofr merged commit 3133716 into master Feb 17, 2017
2 checks passed
2 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
security/snyk No new vulnerabilities
Details
@fofr fofr deleted the custom-org-images branch Feb 17, 2017
andrewgarner added a commit to alphagov/whitehall that referenced this pull request Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.