ChoiceFieldGroup: use <li> instead of <div>, add list-style: none to ms-ChoiceGroup-list class

[jimbarrett33]

Change the <ul> child to be <li> from <div> to be compliant with
HTML spec. Add list-style: none to ms-ChoiceGroup-list class so the
default li bullets won't show up

[msftclas]

Hi @jbarr33, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

[Jahnp]

Approved

Thanks so much for the PR, @jbarr33!

[mikewheaton]

To be clear, this fixes the documentation on GitHub but not the Fabric JS website, right? We should probably be removing the GitHub documentation entirely now that we have the website online.

[Jahnp]

I thought the website was generated from the local documentation?

[jimbarrett33]

@mikewheaton Not sure I understand the about the Fabric JS website or not but if people follow updated documentation which uses li without the corresponding sass/css fix then the code will render the default li bullets which is not good. In that case it would be better to leave the docs as div

[mikewheaton]

@Jahnp: The GitHub documentation was added as a temporary solution while we were trying to get the website built and deployed. It has images of components and code samples but, as far as I know, it isn't related to the actual documentation site in any way. @leddie24 can confirm.

[msftgits]

Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!

[msftclas]

Hi @jbarr33, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

[Jahnp]

@mikewheaton ah, I didn't look closely enough at the actual files affected in this PR. You're right, this PR affects the ghdocs sample code, which I don't think is what shows up on the site. That said, the fix is definitely valid, and IMO we should pull it in for now.

But, we should also fix the code on the website. I'm honestly not familiar enough with where the below code sample is coming from in terms of the JS repo:

@leddie24 @battletoilet can you please take a look and let us know if the changes in this PR will end up on the website? These changes are just to the .md files.

[leddie24]

hey @Jahnp, the HTML code samples are built from the .hbs files inside of ./src/components/ComponentName/ComponentName.hbs. ChoiceFieldGroup is generating the options inside of ms-ChoiceFieldGroup-list as RadioButton components.

ChoiceFieldGroup

  <div class="ms-ChoiceFieldGroup-title">
    <label class="ms-Label{{#if props.required}} is-required{{/if}}">{{props.title}}</label>
  </div>
  <ul class="ms-ChoiceFieldGroup-list">
  {{~#each props.fields}}
    {{renderPartial component props}}
  {{~/each}}
  </ul>
</div> 

RadioButton

  <input tabindex="-1" type="radio" class="ms-RadioButton-input">
  <label role="{{props.type}}"
    class="ms-RadioButton-field{{#if props.disabled}} is-disabled{{/if}}"
    tabindex="0"
    aria-checked="{{props.checked}}"
    name="{{props.name}}"
    {{~#if props.disabled}}aria-disabled="true"{{~/if}}>
    <span class="ms-Label">{{props.label}}</span>
  </label>
</div>

From my understanding, I don't think RadioButton is being used in any other component besides ChoiceFieldGroup, so we can just edit the .hbs template for that component to show LIs instead of divs to fix this issue.

[leddie24]

Made a quick PR for this, it's in #285

[mikewheaton]

Approved

[Jahnp]

Thanks all!