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

chore: migrated filter component and fix some issues with composite-c… #20

Merged
merged 10 commits into from
Jan 31, 2023
Merged

chore: migrated filter component and fix some issues with composite-c… #20

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1e93e1f
chore: migrated filter component and fix some issues with composite-c…
aizad-deriv Jan 9, 2023
cbeedae
fix: resolved issues and update code
aizad-deriv Jan 12, 2023
1fbda5e
Merge branch 'Improvement/72232/Reports_TS_migration' of github.com:m…
aizad-deriv Jan 19, 2023
5cc8a63
fix: change export to CalendarIcon on index.js and added RootStore ty…
aizad-deriv Jan 26, 2023
1285ce4
fix: change TRootStore to RootStore
aizad-deriv Jan 26, 2023
5988b7d
Merge branch 'Improvement/72232/Reports_TS_migration' of github.com:m…
aizad-deriv Jan 26, 2023
540ad3d
fix: replace RootStore to TRootStore
aizad-deriv Jan 26, 2023
917b635
Merge branch 'Improvement/72232/Reports_TS_migration' of github.com:m…
aizad-deriv Jan 30, 2023
cf555fa
Merge branch 'Improvement/72232/Reports_TS_migration' of github.com:m…
aizad-deriv Jan 31, 2023
208c1a9
fix: expand object for filtered_date_range
aizad-deriv Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/reports/src/Components/Form/CompositeCalendar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
import CalendarIcon from './calendar-icon';

export default CalendarIcon;
export default from './composite-calendar.jsx';
Copy link

Choose a reason for hiding this comment

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

We had eslint errors for this pattern so you should not use it

Suggested change
export default from './composite-calendar.jsx';
import CompositeCalendar from './composite-calendar.jsx';
export default CompositeCalendar

25 changes: 12 additions & 13 deletions ...ports/src/Components/filter-component.jsx → ...ports/src/Components/filter-component.tsx
View file
Open in desktop
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;
Copy link
Owner

Choose a reason for hiding this comment

The 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'),
Expand Down Expand Up @@ -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) => ({
Copy link

Choose a reason for hiding this comment

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

Suggested change
export default connect(({ modules }: any) => ({
export default connect(({ modules }: RootStore) => ({

Copy link

Choose a reason for hiding this comment

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

Declare RootStore in the top of the file: import RootStore from '../Stores/index';

Copy link

Choose a reason for hiding this comment

The 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will change this 🙏🏼 to RootStore thanks @akmal-deriv @niloofar-deriv

Copy link
Owner

Choose a reason for hiding this comment

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

@aizad-deriv please change the rootstore type to the Farzin's suggestion
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.

Copy link

Choose a reason for hiding this comment

The 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 😅
Can we merge that PR if there is no concern about it so others can update their branches and use the TRootStore?

cc: @aizad-deriv @niloofar-deriv

Copy link
Owner

Choose a reason for hiding this comment

The 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.

Copy link

Choose a reason for hiding this comment

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

Yes I meant for @deriv/reports TS migration, Awesome, Thank you @mahdiyeh-deriv 🌹

Copy link

Choose a reason for hiding this comment

The 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,
Expand Down
2 changes: 1 addition & 1 deletion packages/reports/src/Containers/statement.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { connect } from 'Stores/connect';
import { getStatementTableColumnsTemplate } from '../Constants/data-table-constants';
import PlaceholderComponent from '../Components/placeholder-component.jsx';
import AccountStatistics from '../Components/account-statistics.jsx';
import FilterComponent from '../Components/filter-component.jsx';
import FilterComponent from '../Components/filter-component';
import { ReportsMeta } from '../Components/reports-meta.jsx';
import EmptyTradeHistoryMessage from '../Components/empty-trade-history-message.jsx';

Expand Down