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

Akmal / Fix: do not close forex markets #1271

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

Akmal / Fix: do not close forex markets #1271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions src/components/SymbolSelectButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,13 @@ export const SymbolInfo = observer(SymbolInfoBase);

const SymbolSelectButtonBase = () => {
const { chartTitle } = useStores();
const { isSymbolOpen } = chartTitle;
const { isSymbolOpen, currentActiveCategory } = chartTitle;
const is_forex_market = currentActiveCategory === 'forex';
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 👌🏻

</div>
<ArrowIcon className='cq-symbol-dropdown' />
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/components/categoricaldisplay/Item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) ? (
Copy link

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

''
) : (
<span className='closed-market'>{t.translate('CLOSED')}</span>
Expand Down