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

Akmal / Fix: do not close forex markets #1271

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akmal-deriv
Copy link
Contributor

No description provided.

@@ -31,7 +31,7 @@ const ItemName = React.memo(({ item: { itemId, display } }: { item: TSubCategory
const ItemDetail = React.memo(
({ favoritesId, item: { dataObject, itemId } }: { favoritesId: string; item: TSubCategoryDataItem }) => (
<div className='sc-mcd__item__detail'>
{dataObject && (dataObject.exchange_is_open === undefined || dataObject.exchange_is_open) ? (
{dataObject && (dataObject.market === 'forex' || dataObject.exchange_is_open === undefined || dataObject.exchange_is_open) ? (

Choose a reason for hiding this comment

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

Suggested change
{dataObject && (dataObject.market === 'forex' || dataObject.exchange_is_open === undefined || dataObject.exchange_is_open) ? (
{dataObject && (dataObject.market === 'forex' || dataObject.exchange_is_open === undefined || dataObject.exchange_is_open) ? (

just a question. are we specifically checking for undefined? looks kinda sus 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes 😁 It was added a long time ago as well 😄

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 think the point of this was that it should only show CLOSED when the market is explicitly closed, and not when the response is undefined/not received yet

return (
<div className='cq-symbol-select-btn'>
<div className='cq-symbol-select-btn__container'>
<SymbolInfo />
{!isSymbolOpen && <div className='cq-symbol-closed-text'>{t.translate('CLOSED')}</div>}
{!isSymbolOpen && !is_forex_market && <div className='cq-symbol-closed-text'>{t.translate('CLOSED')}</div>}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think sometimes Forex markets can be closed from BO. Will it still show Closed here if it is closed in BO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, no, unfortunately this is required due to forex markets being marked as closed when the daily blackout happens. But in case the markets are closed from BO, the user will not be able to make any trade and it will show the error on the right side. I think the idea of the card is that the forex markets do not close in general, so if trading is stopped for some reason, it still should not be marked as closed on our side.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akmal-binary Looks like Forex market will be closed on Weekends. We still need to show Closed label for it.
Screenshot 2022-03-28 at 12 01 36 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Bala, I'll change the logic accordingly 👌🏻

Copy link

@yashim-deriv yashim-deriv left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants