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.

Sign up for GitHub

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

Jump to bottom

fix(ts): make CSSClasses types consistent #4774

Merged
merged 10 commits into from
Jun 14, 2021
Merged

fix(ts): make CSSClasses types consistent #4774

merged 10 commits into from
Jun 14, 2021
+448 −457

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Jun 4, 2021 •
edited
Loading

Summary

In some cases, we were exporting our CSSClasses types as required for our widgets, while they are in reality optional and partial.

To avoid our users to provide extra logic over our types and to keep it consistent with our usage, the types are now:

  • Exported as optional in the widget.
  • Export as required in the component.

Result

  • The exported types are now correct.
  • When a component is provided, its CSSClasses type will be in the same file, named FootComponentCSSClasses

Next step

  • Provide the same update to the templates types

Ticket

DX-1644

@shortcuts
fix(ts): make CSS classes types consistent

Verified

This commit was signed with the committer’s verified signature.
edolstra Eelco Dolstra
GPG key ID: 8170B4726D7198DE
Verified
Learn about vigilant mode
863b7df
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 4, 2021 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 03a14f3:

Sandbox Source
InstantSearch.js Configuration

shortcuts and others added 2 commits June 4, 2021 17:29
@shortcuts shortcuts force-pushed the fix/ts-cssclasses branch from f4178f2 to 7310de9 Compare June 8, 2021 07:23
@shortcuts shortcuts marked this pull request as ready for review June 8, 2021 07:24
@shortcuts shortcuts requested review from a team, tkrugg and Haroenv and removed request for a team June 8, 2021 07:33
Haroenv
Haroenv reviewed Jun 8, 2021
View reviewed changes
@eunjae-lee eunjae-lee mentioned this pull request Jun 8, 2021
Closed
@Haroenv
Copy link
Contributor

Haroenv commented Jun 8, 2021

can we leave this PR open until the remaining components are merged?

#4767
#4770
#4771

@shortcuts
Copy link
Member Author

can we leave this PR open until the remaining components are merged?

Yes, this make sense!

@shortcuts shortcuts mentioned this pull request Jun 10, 2021
Merged
@Haroenv Haroenv mentioned this pull request Jun 11, 2021
Merged
@shortcuts shortcuts force-pushed the fix/ts-cssclasses branch from c5edb61 to b07301a Compare June 11, 2021 11:53
@shortcuts shortcuts requested a review from Haroenv June 11, 2021 11:54
Haroenv
Haroenv approved these changes Jun 11, 2021
View reviewed changes
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

I think this is good!

@shortcuts shortcuts merged commit 99008a9 into master Jun 14, 2021
@shortcuts shortcuts deleted the fix/ts-cssclasses branch June 14, 2021 07:50
@shortcuts shortcuts mentioned this pull request Jun 16, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Haroenv Haroenv

@tkrugg tkrugg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@shortcuts @Haroenv