Conversation
… 'paymentAccepted' & 'currenciesAccepted' properties. For consistency, also added to description of properties: 'discountCurrency', 'price', 'priceCurrency', 'currency'.
Reversed some changes not needed as already visible on webschemas.org eg. HowToSection now a subtype of ListItem, HowToDirection now a subtype of CreativeWork.
|
Thanks, though not gonna rush this one. There are unconventional currencies other than "crypto" currencies, https://en.wikipedia.org/wiki/Local_exchange_trading_system#Examples that we maybe ought to handle in common manner. |
|
Actually my comment was meant w.r.t. #1866 "Added reference to cryptocurrencies in description" ; not sure what those edits were doing here, was that part of the bit you removed? |
…tion for 'paymentAccepted' & 'currenciesAccepted' properties." This reverts commit 182dd79.
|
Spurious commits from cryptocurrencies work reverted. This PR now only includes changes/enhancements to HowTo examples. Good to go for a merge! |
|
Looking at this with a colleague, we are concerned the steps section is broken, but will look at a followup edit after merging as the other fixes are good. |
| </span>$20</div> | ||
| <div>About <span itemprop="totalTime" content="PT30M">30 minutes</span></div> | ||
| <div>Necessary Items:</div> | ||
| <div itemprop="tool" itemscope itemtype="http://schema.org/HowToTool">Spare tire</div> |
There was a problem hiding this comment.
I believe the name of the HowToTool will not be set with this syntax. The previous way works, or if you remove the itemscope & itemtype.
|
@dbiollo Good point. There are two approaches to getting the name of the HowToTool, and HowToSuppy, to be picked out. |
| </div> | ||
| <strong><span itemprop="name">Change a Flat Tire</span></strong> | ||
| <div>About <span itemprop="estimatedCost" itemscope itemtype="http://schema.org/MonetaryAmount"> | ||
| <meta itemprop="currency" content="USD"></meta> |
There was a problem hiding this comment.
There isn't a close tag, meta is an empty element.
| <div itemprop="tool" itemscope itemtype="http://schema.org/HowToTool">Jack</div> | ||
| <div itemprop="tool" itemscope itemtype="http://schema.org/HowToTool">Wheel wedges</div> | ||
| <div itemprop="supply" itemscope itemtype="http://schema.org/HowToSupply">Flares</div> | ||
| <div itemprop="steps" itemscope itemtype="http://schema.org/ItemList"> |
There was a problem hiding this comment.
The field name is now called "step" - see http://webschemas.org/HowTo
There was a problem hiding this comment.
Also "step" can be CreativeWork, HowToSection, HowToStep, Text. So I don't think ItemList is valid is it?
If the ItemList wrapper is removed then the sections can be steps like so:
<div itemprop="step" itemscope itemtype="http://schema.org/HowToSection">
| <meta itemprop="position" content="2"></meta> | ||
| <div itemprop="text">You're going to need space and want to be visible.</div> | ||
| </div> | ||
| <div itemprop="itemListElement" itemscope itemtype="http://schema.org/HowToDirection"> |
There was a problem hiding this comment.
I believe this changes the meaning of the HowTo Steps. Originally there was a HowToStep wrapping each HowToDirection. My understanding is that a HowToStep represents a (usually numbered) significant step, which can contain the HowToDirection primary text and optionally a HowToTip (a side note for more complex steps).
Was there any intent / discussion around changing the design of the HowTo for this release? If not, I'd suggest keeping the original HowToStep structure (wrapping the direction + tip) for now.
| <div itemprop="itemListElement" itemscope itemtype="http://schema.org/HowToSection"> | ||
| <div itemprop="name">Preparation</div> | ||
| <meta itemprop="position" content="1"></meta> | ||
| <div itemprop="steps" itemscope itemtype="http://schema.org/HowToStep"> |
There was a problem hiding this comment.
HowToSection does not have a "steps" property, but it is an ItemList so this itemprop should be "itemListElement".
|
Took a closer look and left some comments on things that look problematic. |
|
Changes made and now in PR (#1902) |
Implements fix for issue (#1884)