-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: migrated filter component and fix some issues with composite-c… #20
Changes from 2 commits
1e93e1f
cbeedae
1fbda5e
5cc8a63
1285ce4
5988b7d
540ad3d
917b635
cf555fa
208c1a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
import CalendarIcon from './calendar-icon'; | ||
import CompositeCalendar from './composite-calendar.jsx'; | ||
|
||
export default CalendarIcon; | ||
export default CompositeCalendar; | ||
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,18 +1,27 @@ | ||||||
import React from 'react'; | ||||||
import PropTypes from 'prop-types'; | ||||||
import { FilterDropdown } from '@deriv/components'; | ||||||
import { localize } from '@deriv/translations'; | ||||||
import { connect } from 'Stores/connect'; | ||||||
import CompositeCalendar from './Form/CompositeCalendar'; | ||||||
|
||||||
type TFilterComponent = { | ||||||
action_type: string; | ||||||
date_from: number; | ||||||
date_to: number; | ||||||
filtered_date_range: object; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better to expand this object and add keys |
||||||
handleDateChange: () => void; | ||||||
handleFilterChange: () => void; | ||||||
suffix_icon: string; | ||||||
}; | ||||||
|
||||||
const FilterComponent = ({ | ||||||
action_type, | ||||||
date_from, | ||||||
date_to, | ||||||
handleFilterChange, | ||||||
handleDateChange, | ||||||
filtered_date_range, | ||||||
}) => { | ||||||
}: TFilterComponent) => { | ||||||
const filter_list = [ | ||||||
{ | ||||||
text: localize('All transactions'), | ||||||
|
@@ -58,17 +67,7 @@ const FilterComponent = ({ | |||||
); | ||||||
}; | ||||||
|
||||||
FilterComponent.propTypes = { | ||||||
action_type: PropTypes.string, | ||||||
date_from: PropTypes.number, | ||||||
date_to: PropTypes.number, | ||||||
filtered_date_range: PropTypes.object, | ||||||
handleDateChange: PropTypes.func, | ||||||
handleFilterChange: PropTypes.func, | ||||||
suffix_icon: PropTypes.string, | ||||||
}; | ||||||
|
||||||
export default connect(({ modules }) => ({ | ||||||
export default connect(({ modules }: any) => ({ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Declare RootStore in the top of the file: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please checkout this as well https://github.com/mahdiyeh-deriv/deriv-app/pull/37/files#r1069147497 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will change this 🙏🏼 to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aizad-deriv please change the rootstore type to the Farzin's suggestion There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mahdiyeh-deriv there's a PR regarding the root store here by Sergei which has not yet been merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mahdiyeh-deriv I believe this is already done by @sergei-deriv in #40, And I think he has also done it in another PR 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @farzin-deriv that PR as circle-ci issue, I'v asked sergei to fix that an I will merge it to the current branch we have for the reports then we can you it for reports migration. About using this changes in other packages @niloofar-deriv could help more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I meant for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure will help to changing them |
||||||
action_type: modules.statement.action_type, | ||||||
data: modules.statement.data, | ||||||
date_from: modules.statement.date_from, | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you change this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the file should export
CompositeCalendar
since it's in the respective folder, but mistakenly it was written to export the wrong component, so the issue is being fixed here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my bad, I'll change it 🙏🏼