Skip to content

[Ashes] Move DateTime to utils - #2228

Merged
tonypizzicato merged 12 commits into
feature/ashes-styleguide-phase-3from
refactor/move-datetime-to-utils
Jun 26, 2017
Merged

[Ashes] Move DateTime to utils#2228
tonypizzicato merged 12 commits into
feature/ashes-styleguide-phase-3from
refactor/move-datetime-to-utils

Conversation

@Bad-Company

@Bad-CompanyBad-Company commented Jun 16, 2017

Copy link
Copy Markdown
Contributor

What was done:

  • datetime.jsx moved to /utils

  • replace propTypes with Flow

  • add components to Styleguide

  • add tests

Conflicts:
ashes/src/components/gift-cards/gift-card.jsx
ashes/src/components/rmas/helpers.jsx
ashes/src/components/table/cell.jsx
ashes/styleguide/config.styleguide.js
*
* @function Moment
*/
export const Moment = ({

@tonypizzicatotonypizzicatoJun 19, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's get rid of exporting Moment to the outside world(also specs and examples from styleguide) to restrict ability to use different formats. looks like it's not used anywhere(https://github.com/FoxComm/highlander/blob/master/ashes/src/components/table/cell.jsx#L34 is the only place, but looks like it is unreachable, can you check it one more time?)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

yep, looks like there is no type: 'moment'used in the project.


it('should render date/time in (L LTS) format by default', function () {
const date = '2017-12-29T15:10:30';
const expectedResult = moment.utc(date).local().format('L LTS');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like this expression is an internal of Moment itself, so expect(datetime.text()).to.equal(expectedResult); asserts that internals of Moment equal to the result of <Moment> call.
i think it should be smth like

const date = '2017-12-29T15:10:30';
const expectedResult = '12/29/2017 3:10 PM';
const datetime = mount(
<DateTime.Moment value={date} />
);
expect(datetime.text()).to.equal(expectedResult);


type Props = {
/** time string */
value?: string,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

required field

};


export const DateTime = (props: Props) => <Moment {...props} format={'L LT'} />;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these components props type is not the same as Moment's props - it doesn't accept format

type DateTimeProps = {
value: string,
utc?: boolean,
emptyValue?: string,
className?: string
}
type MomentProps = DateTimeProps & {
format: string,
};

#### Basic usage

```javascript
<DateTime value={1497636772107} />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Props type of Moment says that value type is string

@tonypizzicato
tonypizzicato merged commit 844498e into feature/ashes-styleguide-phase-3Jun 26, 2017
@tonypizzicato
tonypizzicato deleted the refactor/move-datetime-to-utils branch June 26, 2017 11:19
@DiokuzDiokuz mentioned this pull request Jun 27, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Bad-Company@tonypizzicato@eugene-sy