Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APP-3274 - Update React imports to use standard style. #68

Merged
merged 5 commits into from
Nov 13, 2020
Merged

APP-3274 - Update React imports to use standard style. #68

merged 5 commits into from
Nov 13, 2020

Conversation

sasha-symphony
Copy link
Contributor

Description

When adding uitoolkit-components as a dependency to SFE-Lite, typescript complains about the use of

import React from 'react';

This PR updates all components to use the preferred

import * as React from 'react';

Links

PR in react repo establishing this as standard.

JIRA-ticket

@sasha-symphony sasha-symphony requested a review from a team as a code owner November 12, 2020 16:48
@sasha-symphony sasha-symphony self-assigned this Nov 12, 2020
@symphony-adnane
Copy link
Contributor

lint is failing @sasha-symphony

@@ -47,7 +47,7 @@ describe('Key Utils', () => {
it(`should handle ${test.key} shift=${test.shiftKey} on cell`, () => {
const event = createEvent({ key: test.key, shiftKey: test.shiftKey });
const wrapper = shallow(
<div onKeyDown={(e) => handleKeyDownCell(new Date(), e, setMethod, setMethod2, {})} />
<div onKeyDown={(e) => handleKeyDownCell(new Date(), e, setMethod, setMethod2, ()=>{}, {})} />
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a mistake? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. handleKeyDownCell takes 6 params, but this test was only supplying 5, and the 5th one was wrong. I simply added an empty function for focusDiv. Perhaps some of the types are supposed to be optional. The locale isn't typed at all.

export function handleKeyDownCell(
  date: Date,
  e: React.KeyboardEvent,
  setNavigationDate: (date) => any,
  setDivToFocus: (date) => any,
  focusDiv: (date) => any,
  locale
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted this change, as it isn't technically within the scope of this PR, but it should be fixed.

The error I was attempting to fix was a missing parameter. If any of the parameters are optional, they should be labelled as such, and the locale param should have a type. @AliciaSymphony

@symphony-adnane symphony-adnane self-requested a review November 12, 2020 17:21
@symphony-adnane symphony-adnane merged commit 4068649 into SymphonyPlatformSolutions:master Nov 13, 2020
@sasha-symphony sasha-symphony deleted the fix-imports branch November 13, 2020 17:23
@ghost ghost unassigned sasha-symphony Mar 15, 2021
symphony-jean-michael referenced this pull request in symphony-jean-michael/symphony-bdk-ui-toolkit-components Nov 4, 2021
APP-3785 Difference option and tk-option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants