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

Add opengraph metadata #2238

Closed
wants to merge 2 commits into from
Closed

Add opengraph metadata #2238

wants to merge 2 commits into from

Conversation

aaronmgdr
Copy link
Member

Description

Add full Open Graph meta data to brand Kit pages

add open graph image to /build/faucet /build/wallet

Tested

confirmed the title and description, should test image shows correct once not on localhost

Other changes

Describe any minor or "drive-by" changes here.

Related issues

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Should we have these metadata tags on every page? If so, should they be added to the Page component?

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@a525213). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2238   +/-   ##
=========================================
  Coverage          ?   74.81%           
=========================================
  Files             ?      276           
  Lines             ?     7766           
  Branches          ?      976           
=========================================
  Hits              ?     5810           
  Misses            ?     1839           
  Partials          ?      117
Flag Coverage Δ
#mobile 74.81% <ø> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a525213...71cca1e. Read the comment docs.

@jmrossy jmrossy changed the title Add opengrpah meta data Add opengraph metadata Dec 14, 2019
@aaronmgdr
Copy link
Member Author

Should we have these metadata tags on every page? If so, should they be added to the Page component?

@jmrossy Im not clear on what you are asking/proposing.

We do want each page in the brandkit and most other pages to have the metatags. They are mostly different per page, the brandkit pages do share a preview image however.

note the component is only for brandkit experience pages.

@aaronmgdr aaronmgdr mentioned this pull request Dec 17, 2019
@jmrossy
Copy link
Contributor

jmrossy commented Dec 17, 2019

Should we have these metadata tags on every page? If so, should they be added to the Page component?

@jmrossy Im not clear on what you are asking/proposing.

We do want each page in the brandkit and most other pages to have the metatags. They are mostly different per page, the brandkit pages do share a preview image however.

note the component is only for brandkit experience pages.

@aaronmgdr My thought was to have these be mandatory props of for each Page wrapper if we wanted them on every page. If we don't then nevermind :)

@jmrossy
Copy link
Contributor

jmrossy commented Dec 17, 2019

Assigning back as tests are failing

@jmrossy jmrossy assigned aaronmgdr and unassigned jmrossy and cmcewen Dec 17, 2019
@aaronmgdr
Copy link
Member Author

closing as @cmcewen included these changes in #2279

@aaronmgdr aaronmgdr closed this Dec 17, 2019
@aaronmgdr aaronmgdr deleted the aaronmgdr/opengraph branch January 2, 2020 18:37
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.

Add Open Graph data and images
3 participants