Skip to content
This repository was archived by the owner on Nov 9, 2021. It is now read-only.

Adding BreadcrumbList schema.org spec#119

Open
kiwiupover wants to merge 1 commit intopoteto:masterfrom
kiwiupover:schema-style
Open

Adding BreadcrumbList schema.org spec#119
kiwiupover wants to merge 1 commit intopoteto:masterfrom
kiwiupover:schema-style

Conversation

@kiwiupover
Copy link
Copy Markdown

@kiwiupover kiwiupover commented May 25, 2017

Closes #118

Usage:

{{bread-crumbs hasSchema=true}}

TODO

  • make decision on where to do the configuration
    1. use the config.environment
    2. use the current solution and find and alternative place to reopen the LinkComponent

https://developers.google.com/search/docs/data-types/breadcrumbs
http://schema.org/BreadcrumbList

output from foo/bar/baz route

Validate the output here https://search.google.com/structured-data/testing-tool

<ol id="hasSchema" itemscope="" itemtype="http://schema.org/BreadcrumbList" class="ember-view breadcrumb">
  <li id="ember469" itemprop="itemListElement" itemscope="" itemtype="http://schema.org/ListItem" class="ember-view">
    <a id="ember470" href="/foo" itemscope="" itemtype="http://schema.org/Thing" itemprop="item" class="ember-view"> <span itemprop="name">
            I am Foo Index
          </span>
    </a>
    <meta itemprop="position" content="1">
  </li>
  <li id="ember472" itemprop="itemListElement" itemscope="" itemtype="http://schema.org/ListItem" class="ember-view">
    <a id="ember473" href="/foo/bar" itemscope="" itemtype="http://schema.org/Thing" itemprop="item" class="ember-view active"> <span itemprop="name">
            I am Bar
          </span>
    </a>
    <meta itemprop="position" content="2">
  </li>
  <li id="ember475" itemprop="itemListElement" itemscope="" itemtype="http://schema.org/ListItem" class="ember-view"> <span itemprop="name">
        I am Baz
      </span>
    <meta itemprop="position" content="3">
  </li>
</ol>

@trabus thanks for your help!!

@kiwiupover
Copy link
Copy Markdown
Author

kiwiupover commented May 25, 2017

@poteto
I thinks this is a good start. There are a couple of issues which I will comment in the code.

Is there no CI?

Copy link
Copy Markdown
Author

@kiwiupover kiwiupover left a comment

Choose a reason for hiding this comment

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

I will update the readme when this is ready to land and we have a final API.

Cheers


init() {
this._super(...arguments);
const hasSchema = get(this, 'breadCrumbs.hasSchema');
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We need to do this in two places we could move it a mixin.

{{#if hasBlock}}
{{yield this route}}
{{else}}
{{#if hasSchema}}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We could make one more component to clean up this level of nest ifs

{{else}}
{{route.title}}
{{/if}}
{{#link-to route.path class=linkClass itemscope=true itemtype="http://schema.org/Thing" itemprop="item"}}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

FYI These attrs it the link-to with not be compiled if we don't reopen the LinkComponent

Have you thought about using the href-to addon here?

init() {
this._super(...arguments);

LinkComponent.reopen({
Copy link
Copy Markdown
Author

@kiwiupover kiwiupover May 25, 2017

Choose a reason for hiding this comment

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

We need to reopen the LinkComponent to add attributeBindings but need to do this behind the hasSchema flag. However that is too late in the code execution path.

Also to make this backward compatible we need a version flag to switch to LinkView in pre 2.10 apps.

Do you have thoughts about?
We could move the configuration to the ENV and reopen the LinkComponent in the initializer

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant