-
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
refactor: filter modal refactoring, ts migration #67
refactor: filter modal refactoring, ts migration #67
Conversation
}); | ||
it('should handle toggle button click', () => { | ||
render(<FilterModalBody {...mock_props} />); | ||
userEvent.click(screen.getByRole('checkbox')); |
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.
can we specify the name of the checkbox here?
{show_filter_payment_methods ? ( | ||
<React.Fragment> | ||
<FilterModalSearch /> | ||
<div className='filter-modal__checkbox-container'> |
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.
Please rename the classnames to use the component name.
<div className='filter-modal__checkbox-container'> | |
<div className='filter-modal-body__checkbox-container'> |
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.
do we need a separate css file as well? didn't change it since i didn't create separate files for these.
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.
we can use filter-modal css for this
primary | ||
onClick={onClickConfirmPaymentMethods} | ||
> | ||
{localize('Confirm')} |
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.
Please use Localize
since localize
function has rendering issues.
{localize('Confirm')} | |
<Localize i18n_default_text='Confirm' /> |
) : ( | ||
<Button.Group className={class_name}> | ||
<Button large secondary onClick={onClickReset}> | ||
{localize('Reset')} |
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.
{localize('Reset')} | |
<Localize i18n_default_text='Reset' /> |
{localize('Reset')} | ||
</Button> | ||
<Button disabled={!has_made_changes} large primary onClick={onClickApply}> | ||
{localize('Apply')} |
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.
{localize('Apply')} | |
<Localize i18n_default_text='Apply' /> |
@@ -25,7 +29,6 @@ const FilterModalHeader = ({ pageHeaderReturnFn }) => { | |||
className='filter-modal__header-return-button' |
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.
Please rename the classnames to use the component name.
className='filter-modal__header-return-button' | |
className='filter-modal-header__return-button' |
}; | ||
|
||
const FilterModalNoResults = ({ text }: TFilterModalNoResultsProps) => ( | ||
<div className='filter-modal-no-result'> |
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.
<div className='filter-modal-no-result'> | |
<div className='filter-modal-no-results'> |
return () => { | ||
clearSearchSection(); | ||
}; | ||
// eslint-disable-next-line react-hooks/exhaustive-deps |
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.
If possible, please add the dependencies to avoid having this.
}; | ||
|
||
const onSearchKeyUp = submitForm => { | ||
const onSearchKeyUp = (submitForm: VoidFunction) => { | ||
let typing_timer: NodeJS.Timeout | undefined; |
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.
let typing_timer: NodeJS.Timeout | undefined; | |
let typing_timer?: NodeJS.Timeout; |
🤔
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.
not working
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.
How about this?
const onSearchKeyUp = submitForm => {
const typing_timer = setTimeout(() => {
submitForm();
}, 1000);
clearTimeout(typing_timer);
};
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.
wont the functionality change then if we change the order?
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.
it shouldn't right. Because we clean the timeout, so why we clean it before we even initialize it?
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.
it can be undefined before initializing but then it will change the value right.. so shouldn't it clear any previously set timeout values if there are any?
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.
there will be no previous value because the variable typing_timer
scope is only for this function? 🤔
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.
@farhan-nurzi-deriv the way Nada wrote this is correct, if you check the docs and examples they all doing the same 🤔
Did you tried your suggestion? Did it work?
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.
My sugegestion doesnt works and initial implementation also doesnt works because it doesnt really do cleartimeout. typing_timer is always undefined when we call clearTimeout , we need to move it out outside of the function 👁️ 👁️
key: 'LeavePageModal', | ||
props: { | ||
onLeavePage: () => { | ||
if (isMobile()) { |
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.
Please use is_mobile from ui store instead of this
This one was deprecated
@@ -39,26 +39,26 @@ const ModalManagerContextProvider = (props: React.PropsWithChildren<{ mock?: TMo | |||
* @param {key} string - the key to specify when persisting the local state, by default you should specify the local state name | |||
* @param {default_state} - the value you want the state to be initially | |||
*/ | |||
const useSavedState = (key, default_state) => { | |||
const useSavedState = (key: string, default_state: string[]) => { |
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.
non-blocking:
i changed a bit here as well and fixed the typing issue for it but i dont think it'll cause any huge conflicts when we merge quick add modal as well 🙏
@@ -0,0 +1,65 @@ | |||
import React from 'react'; | |||
import { Button } from '@deriv/components'; | |||
import { localize, Localize } from 'Components/i18next'; |
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 don't think localize
is still being used here. We can remove it.
chore: remove unwanted test case
…nts (binary-com#13504) * integrated manual-flow-components * chore: renamed file * fix: added manual upload in route * fix: dropzone upload issue * chore: added selfie-docment-upload section * fix: icon color and component * chore: added selfieupload form * [COJ] Shaheer / COJ-506 / Manual flow integration (#63) * fix: uses text component from deriv ui * fix: fixes test * fix: fixes styling * fix: fixes button * test: adds tests for manual form * fix: checks country code * fix: prevents unrequired items from hook * style: adds full height * fix: turns off autocomplete * feat: adds yup validations * fix: fixes type * refactor: refactor file upload * refactor: updates config * Likhith/manual form code clean up (#64) * fix: included manual-form changes * fix: updated selfie image * fix: removed unwanted props * fix: removed unwanted props * fix: removed commented code * Fixes datepicker component (#65) * fix: uses text component from deriv ui * fix: fixes test * fix: fixes styling * fix: fixes button * test: adds tests for manual form * fix: checks country code * fix: prevents unrequired items from hook * style: adds full height * fix: turns off autocomplete * feat: adds yup validations * fix: fixes type * refactor: refactor file upload * refactor: updates config * fix: included manual-form changes * fix: updated selfie image * fix: removed unwanted props * fix: removed unwanted props * refactor: moves date picker component out * refactor: refactors datepicker * refactor: refactors date picker * refactor: fixes type * refactor: refactors datepicker * refactor: renames component * fix: passes props down the road * refactor: renames testid * test: fixes test --------- Co-authored-by: Likhith Kolayari <likhith@regentmarkets.com> * fix: integrateion of manual-flow * fix: manual upload date pick * fix: added testcases for manual * fix: code smells * fix: fixes validation default * fix: persists file dropzone * Shaheer/coj 506 (#67) * fix: fixes validation default * fix: persists file dropzone * fix: validate form on mount * fix: fixes height * fix: validation errors * fix: type issues * fix: testcases * fix: testcases * fix: fixes nimc text error * fix: removes console statements * fix: adds todo for useKycAuthStatus hook * test: fixes test case * fix: adds color class --------- Co-authored-by: Shaheer <122449658+shaheer-deriv@users.noreply.github.com> Co-authored-by: Shaheer <shaheer@deriv.com>
* chore: empty commit * feat: tabs + background + initialized EmptyMessage component * Maryia/Positions-redesign/improve EmptyMessage component + add tests (#50) * feat: redirect to trade upon button click on the empty page * test: EmptyMessage * chore: empty commit * DTRA-1279 / Kate / Filter [WIP] (#51) * feat: add dropdowm and action sheet * feat: add apply functionality * chore: remove unused functionality * refactor: separate apply logic * feat: add clear all functionality * feat: apply filtration logic * fix: filtration bug * chore: rename variables * refactor: extract filtration logic into a util function * chore: empty commit * fix: sonarcloud issues * chore: empty commit * chore: add modules store and useclosedposition hook (#52) * refactor: remove todo and change some prop based on quill updates (#54) * DTRA-1279 / Kate / Use hook for real data (#55) * feat: use hook for real data * refactor: apply suggestions * Maryia/positions-redesign/Contract cards [WIP] (#53) * feat: init ContractCard * feat: Contract cards * feat: use ReportsStoreProvider * style: remove unnecessary comment * fix: key * fix: remove old card styles * fix: types * fix: use contract_info in filterPositions * fix: do not show buttons for sold contracts * DTRA-1279 / Kate / Refactor handling open and closed positions and their filtration (#56) * refactor: move stor values to poitions content file * chore: remove code smell * Maryia/positions-redesign/Contract cards improvements + fetching Open positions + formatProfitTableTransactions TS migration (#58) * refactor: contract-card-list and card * feat: buttons demo + animation improvements * feat: finilize Duration component for the card * chore: ts migration for closed positions * fix: console error with remaining time & showing empty message only when empty * feat: connect real open postions + style and filter fixes * fix: style * DTRA-1279/ Kate / Create filter component (#57) * feat: create new filter component * feat: apply radio button * refactor: default time filter logic * feat: add time filtration * refactor: remove unused css * DTRA-1279 / Kate / Refactor: add new content for empty page (#59) * refactor: add new content for empty page * chore: test text update * Maryia/positions-redesign/Contract cards data update fix (#61) * fix: Accumulators tick passed count * fix: contract cards update * fix: show loading only when should not show empty message or cards * chore: update quill version (#63) * Maryia/positions-redesign/Contract card loading state and status timer updates + EmptyPositions update (#64) * feat: loading functionality + fix for status timer * chore: update copy for empty-positions * revert: use hasActionButtons prop instead of impicit onClose * DTRA-1279/ Kate/ Feat: add Date picker (#62) * feat: add second action sheet * feat: add date range formatting and refactored existing code * feat: add range selection filtration * refactor: chip and time filter * fix: empty posituions after filtration * refactor: do clean up * chore: rename variables * chore: localization * DTRA-1279 / Kate/ Add filtration hooks (#65) * feat: create hooks * refactor: rename methods * DTRA-1279 / Kate / Add tests (#66) * refactor: add tests for chip component * refactor: add tests for date picker * refactor: add tests for contract type filter * refactor: add tests for custom time filter button * refactor: add tests for positions utils * DTRA-1279 / Kate / Double filtration and extra filter options (#67) * fix: filtration for today and yersterday * fix: double filter * refactor: change style after design confirmation and sort props * refactor: start adding tets for time filter * chore: apply suggestions * chore: update quill and token library version * DTRA-1279 / Kate / Add section separator (#68) * feat: add sections with date * feat: make filter always visible * refactor: style for date separator * refactor: format time function * refactor: add tests * chore: remove unused wrapper * chore: apply suggestions * DTRA-1279 / Kate / Tech Debt (#69) * refactor: add more test cases for time-filter * refactor: add tests for hooks * refactor: removed some todos * Maryia/positions-redesign/finilise contract card + add total profit loss + initiate pagination in closed positions (#70) * refactor: utilize Tag in ContractCardStatusTimer * chore: add opacity transition to buttons when revealing/hiding them * feat: add total profit + improve card * fix: card deletion transition + total pnl positioning * feat: add pagination on scroll (initial version) * fix: loading state and loading more on infinite scroll in Closed tab (#71) * DTRA-1279 / Kate / Tech Debt part 2 [WIP] (#72) * refactor: add tests for utils functions + removed unused hook * refactor: move total profit loss to a separate folder and add tests * refactor: add tests for positions * refactor: add tests for position content file * Maryia/positions-redesign/test contract card + fix scroll behavior, dates formatting, and filtering Closed positions (#73) * test: contract-card * fix: hide filters on scroll + utilize moment for formatting date in closed tab * refactor: update quill version and refactor * refactor: chip component (#74) * refactor: position content page * Maryia/positions-redesign/test: ContractCardList, ContractCardStatusTimer, PositionsStore, getCurrentTick() + refactoring (#75) * test: contract-card-list * test: ContractCardStatusTimer * test: getCurrentTick() in contract.tsx in shared * test: PositionsStore * test: add more tests to ContractCardList * refactor: desctructure props in mocked component * Maryia/positions-redesign/fix: tests + address sonarcloud + use clsx (#76) * fix: tests + address sonarcloud * refactor: use clsx instead of classnames * refactor: sonarcloud - reduce complexity * fix: cards filtering in PositionsContent + tests + style+bug fixes * build: trigger checks * fix: hasNoActiveFilters condition * fix: update package version and remove prop from action sheet * chore: rename function * Maryia/positions-redesign/feat: display correct active positions count (binary-com#77) * feat: display correct active positions count * fix: BottomNav tests * refactor: filter behaviour * chore: add padding * fix: positions count in footer to not show 0 (binary-com#78) * refactor: total profit loss * chore: add tests for tpl and refactor date picker * refactor: add loadre inside of empty positions * fix: tests * Maryia/positions-redesign/fix: loader on infinite scroll in Closed tab + make redirectTo prop optional in ContractCard (binary-com#79) * fix: place loading after contract cards sections * chore: make redirection optional when clicking on contract card * chore: rename timet * DTRA-1279 / Kate / Add a single date selection (binary-com#80) * feat: add partial range * refactor: tests * refactor: callback --------- Co-authored-by: kate-deriv <kate@regentmarkets.com> Co-authored-by: kate-deriv <121025168+kate-deriv@users.noreply.github.com> Co-authored-by: balakrishna-deriv <56330681+balakrishna-deriv@users.noreply.github.com>
Changes:
ts migration, unit test
Screenshots:
Please provide some screenshots of the change.