Skip to content
This repository was archived by the owner on Aug 7, 2020. It is now read-only.

Conversation

AxelPeter
Copy link
Contributor

  • Add CodeOwners and Github files
  • Standardize all documentation files
  • Fix provider methods
  • Add transclude support
  • Fix component templates

Copy link
Contributor

@euhmeuh euhmeuh left a comment

Choose a reason for hiding this comment

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

Yay!

@AxelPeter AxelPeter force-pushed the docs/standardization branch from 9f8cf88 to 521bc42 Compare August 3, 2018 13:13
@AxelPeter
Copy link
Contributor Author

I have done a rebase with the merge of #242

@@ -0,0 +1 @@
@AxelPeter @FredericEspiau @euhmeuh @JeremyDec
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


**Expected behavior:**

<!-- What you expect to happen-->
Copy link
Contributor

Choose a reason for hiding this comment

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

<-- What did you expect to happen -- >

### Description

<!-- Required -->
<!-- Description of the issue] -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove ]

<!-- Required -->
<!-- Description of the issue] -->

### Steps to Reproduce
Copy link
Contributor

Choose a reason for hiding this comment

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

### Steps to reproduce

2. [Second Step]
3. [and so on...]

**Expected behavior:**
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need the :

### On change

**Note:** Model will not be refreshed until the `on-change` callback as not finished. If you want to access the new model inside the `on-change` callback you need to use the `modelValue` variable as below.
**Note**: Model will not be refreshed until the `on-change` callback as not finished. If you want to access the new model inside the `on-change` callback you need to use the `modelValue` variable as below.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to access the new model inside the ``on-change`` callback you need to use the modelValue variable as below.

->

If you want to access the new model inside the ``on-change`` callback, you need to use the modelValue variable as below.

| on-change | function | &? | | | | handler triggered when value has changed
| Attribute | Type | Binding | One-time Binding | Values | Default | Description
| ---- | ---- | ---- | ---- | ---- | ---- | ----
| `model` | nullable&lt;boolean&gt; | =? | no | `true`, `false`, `null` | n/a | current value of the checkbox and null is considered as `indeterminate`
Copy link
Contributor

Choose a reason for hiding this comment

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

nullable<boolean> -> boolean? to replicate C# behavior https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/nullable-types/?

or

nullable<boolean> -> boolean | nullto replicate TypeScript behaviors https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html ?

import clamp from "lodash/clamp";

// By design, value is restricted to [0, 99999] interval
const MIN_VALUE = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not Number.MIN_VALUE and Number.MAX_VALUE ?

}

$postLink () {
// Sometimes the digest cycle is done before dom manipulation,
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be specified everywhere, maybe have a document to explain the design choices

}

setModelValue (value) {
// updates model and displayed value
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I can read the code myself 😄. The method name says the exact same thing

@AxelPeter AxelPeter force-pushed the docs/standardization branch from 56d9512 to 83869e0 Compare August 3, 2018 15:04
@AxelPeter AxelPeter force-pushed the docs/standardization branch from 83869e0 to ad67796 Compare August 3, 2018 15:08
@AxelPeter AxelPeter merged commit 6e01a80 into develop Aug 3, 2018
@AxelPeter AxelPeter deleted the docs/standardization branch August 3, 2018 15:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Development

Successfully merging this pull request may close these issues.

3 participants