-
Notifications
You must be signed in to change notification settings - Fork 300
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
Adrienne / Show online offline status in advertiser page #6913
Changes from 39 commits
1437840
d7e1392
02f3c81
a27e914
f7f066d
f6533ef
063c0e3
69d5a69
34abed3
f9a9d9e
916e095
77b7179
be053b7
808f17a
0b36af8
f642c6d
7780355
5f43dbc
b9ca64e
a3e01b3
db2a074
25276f9
c4616bc
0b4ee87
e6af86f
78ff79c
b78b159
5f186d2
8a812d6
0f31d36
9f19e52
363c02b
4a08344
70551fc
55dbbbd
272ea17
98d4025
a7ccac4
2f07229
6f3596e
6ff5a05
3f886ca
3de41d4
57a210d
19b070d
9a052bf
0053e10
c1bf364
c1be825
e4052a8
d234256
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 |
---|---|---|
|
@@ -9,6 +9,8 @@ import './advertiser-page.scss'; | |
const AdvertiserPageStats = () => { | ||
const { advertiser_page_store, general_store } = useStores(); | ||
|
||
const is_my_advert = advertiser_page_store.advertiser_details_name === general_store.advertiser_info.name; | ||
const info = is_my_advert ? general_store.advertiser_info : advertiser_page_store.counterparty_advertiser_info; | ||
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. So I found the cause of this issue: This is because we are already subscribed to listen to our own advertiser details within general store, and thus the reason why the websocket is returning Thus the approach I can find at the moment without breaking other pages using
If this approach is not a good one, I could try to create a new computed value to return either |
||
const { | ||
buy_completion_rate, | ||
buy_orders_amount, | ||
|
@@ -19,7 +21,7 @@ const AdvertiserPageStats = () => { | |
sell_completion_rate, | ||
sell_orders_amount, | ||
sell_orders_count, | ||
} = advertiser_page_store.counterparty_advertiser_info; | ||
} = info; | ||
|
||
const avg_buy_time_in_minutes = buy_time_avg > 60 ? Math.round(buy_time_avg / 60) : '< 1'; | ||
const avg_release_time_in_minutes = release_time_avg > 60 ? Math.round(release_time_avg / 60) : '< 1'; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,31 +20,34 @@ import BlockUserOverlay from './block-user/block-user-overlay'; | |
import BlockUserModal from 'Components/block-user/block-user-modal'; | ||
import ErrorModal from 'Components/error-modal/error-modal'; | ||
import classNames from 'classnames'; | ||
import { OnlineStatusIcon, OnlineStatusLabel } from 'Components/online-status'; | ||
adrienne-deriv marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import './advertiser-page.scss'; | ||
|
||
const AdvertiserPage = () => { | ||
const { general_store, advertiser_page_store, buy_sell_store } = useStores(); | ||
|
||
const is_my_advert = advertiser_page_store.advertiser_details_name === general_store.advertiser_info.name; | ||
const info = is_my_advert ? general_store.advertiser_info : advertiser_page_store.counterparty_advertiser_info; | ||
const { | ||
basic_verification, | ||
buy_orders_count, | ||
created_time, | ||
first_name, | ||
full_verification, | ||
id, | ||
is_online, | ||
last_online_time, | ||
last_name, | ||
name, | ||
rating_average, | ||
rating_count, | ||
recommended_average, | ||
recommended_count, | ||
sell_orders_count, | ||
} = advertiser_page_store.counterparty_advertiser_info; | ||
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. When you are viewing your own ads, this will return the previously viewed counterparty advertiser's info instead of your own info |
||
} = info; | ||
|
||
// rating_average_decimal converts rating_average to 1 d.p number | ||
const rating_average_decimal = rating_average ? Number(rating_average).toFixed(1) : null; | ||
const joined_since = daysSince(created_time); | ||
const is_my_advert = id === general_store.advertiser_id; | ||
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.
|
||
const [is_error_modal_open, setIsErrorModalOpen] = React.useState(false); | ||
|
||
React.useEffect(() => { | ||
|
@@ -78,7 +81,8 @@ const AdvertiserPage = () => { | |
return ( | ||
<div | ||
className={classNames('advertiser-page', { | ||
'advertiser-page--no-scroll': !!advertiser_page_store.is_counterparty_advertiser_blocked, | ||
'advertiser-page--no-scroll': | ||
!!advertiser_page_store.is_counterparty_advertiser_blocked && !is_my_advert, | ||
})} | ||
> | ||
<RateChangeModal onMount={advertiser_page_store.setShowAdPopup} /> | ||
|
@@ -96,7 +100,7 @@ const AdvertiserPage = () => { | |
/> | ||
<BlockUserModal | ||
advertiser_name={name} | ||
is_advertiser_blocked={!!advertiser_page_store.is_counterparty_advertiser_blocked} | ||
is_advertiser_blocked={!!advertiser_page_store.is_counterparty_advertiser_blocked && !is_my_advert} | ||
is_block_user_modal_open={ | ||
general_store.is_block_user_modal_open && !general_store.block_unblock_user_error | ||
} | ||
|
@@ -122,7 +126,7 @@ const AdvertiserPage = () => { | |
)} | ||
</div> | ||
<BlockUserOverlay | ||
is_visible={!!advertiser_page_store.is_counterparty_advertiser_blocked} | ||
is_visible={!!advertiser_page_store.is_counterparty_advertiser_blocked && !is_my_advert} | ||
onClickUnblock={() => general_store.setIsBlockUserModalOpen(true)} | ||
> | ||
<div className='advertiser-page-details-container'> | ||
|
@@ -145,7 +149,60 @@ const AdvertiserPage = () => { | |
</div> | ||
)} | ||
</div> | ||
|
||
<MobileWrapper> | ||
<div className='advertiser-page__row'> | ||
<div className='advertiser-page__rating--row'> | ||
<OnlineStatusIcon is_online={is_online} /> | ||
<OnlineStatusLabel is_online={is_online} last_online_time={last_online_time} /> | ||
</div> | ||
<div className='advertiser-page__rating--row'> | ||
<Text | ||
className='advertiser-page__joined-since' | ||
color='less-prominent' | ||
size='xxxs' | ||
> | ||
{joined_since ? ( | ||
<Localize | ||
i18n_default_text='Joined {{days_since_joined}}d' | ||
values={{ days_since_joined: joined_since }} | ||
/> | ||
) : ( | ||
<Localize i18n_default_text='Joined today' /> | ||
)} | ||
</Text> | ||
</div> | ||
</div> | ||
</MobileWrapper> | ||
|
||
<div className='advertiser-page__rating'> | ||
<DesktopWrapper> | ||
<React.Fragment> | ||
<div className='advertiser-page__rating--row'> | ||
<OnlineStatusIcon is_online={is_online} /> | ||
<OnlineStatusLabel | ||
is_online={is_online} | ||
last_online_time={last_online_time} | ||
/> | ||
</div> | ||
<div className='advertiser-page__rating--row'> | ||
<Text | ||
className='advertiser-page__joined-since' | ||
color='less-prominent' | ||
size='xs' | ||
> | ||
{joined_since ? ( | ||
<Localize | ||
i18n_default_text='Joined {{days_since_joined}}d' | ||
values={{ days_since_joined: joined_since }} | ||
/> | ||
) : ( | ||
<Localize i18n_default_text='Joined today' /> | ||
)} | ||
</Text> | ||
</div> | ||
</React.Fragment> | ||
</DesktopWrapper> | ||
{rating_average ? ( | ||
<React.Fragment> | ||
<div className='advertiser-page__rating--row'> | ||
|
@@ -193,37 +250,7 @@ const AdvertiserPage = () => { | |
</Text> | ||
</div> | ||
)} | ||
{!isMobile() && ( | ||
<div className='advertiser-page__rating--row'> | ||
<Text color='less-prominent' size={isMobile() ? 'xxxs' : 'xs'}> | ||
{joined_since > 0 ? ( | ||
<Localize | ||
i18n_default_text='Joined {{days_since_joined}}d' | ||
values={{ days_since_joined: joined_since }} | ||
/> | ||
) : ( | ||
<Localize i18n_default_text='Joined today' /> | ||
)} | ||
</Text> | ||
</div> | ||
)} | ||
</div> | ||
{isMobile() && ( | ||
<Text | ||
className='advertiser-page__joined-since' | ||
color='less-prominent' | ||
size={isMobile() ? 'xxxs' : 'xs'} | ||
> | ||
{joined_since > 0 ? ( | ||
<Localize | ||
i18n_default_text='Joined {{days_since_joined}}d' | ||
values={{ days_since_joined: joined_since }} | ||
/> | ||
) : ( | ||
<Localize i18n_default_text='Joined today' /> | ||
)} | ||
</Text> | ||
)} | ||
<div className='advertiser-page__row'> | ||
<TradeBadge | ||
is_poa_verified={!!full_verification} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import OnlineStatusIcon from './online-status-icon'; | ||
import OnlineStatusLabel from './online-status-label'; | ||
import './online-status.scss'; | ||
|
||
export { OnlineStatusIcon, OnlineStatusLabel }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import React from 'react'; | ||
import classNames from 'classnames'; | ||
import PropTypes from 'prop-types'; | ||
import { observer } from 'mobx-react-lite'; | ||
|
||
const OnlineStatusIcon = ({ is_online, size = '1em' }) => { | ||
return ( | ||
<div | ||
className={classNames('online-status__icon', { | ||
'online-status__icon--offline': !is_online, | ||
'online-status__icon--online': !!is_online, | ||
})} | ||
style={{ | ||
width: size, | ||
height: size, | ||
}} | ||
/> | ||
); | ||
}; | ||
|
||
OnlineStatusIcon.propTypes = { | ||
is_online: PropTypes.oneOfType([PropTypes.number, PropTypes.bool]).isRequired, | ||
size: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), | ||
}; | ||
|
||
export default observer(OnlineStatusIcon); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import React from 'react'; | ||
import { Text } from '@deriv/components'; | ||
import { isMobile } from '@deriv/shared'; | ||
import { localize } from 'Components/i18next'; | ||
import { observer } from 'mobx-react-lite'; | ||
import PropTypes from 'prop-types'; | ||
import moment from 'moment'; | ||
|
||
const OnlineStatusLabel = ({ is_online, last_online_time, size = isMobile() ? 'xxxs' : 'xs' }) => { | ||
const last_online_label = () => { | ||
if (!is_online) { | ||
if (last_online_time) { | ||
const current_date = new Date(); | ||
const last_online_date = new Date(last_online_time * 1000); | ||
const diff = moment.duration(moment(current_date).diff(last_online_date)); | ||
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. can we use |
||
const addPrural = duration => (duration !== 1 ? 's' : ''); | ||
nijil-deriv marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (diff.months()) | ||
return localize('Seen {{ duration }} month{{ prural }} ago', { | ||
duration: diff.months(), | ||
prural: addPrural(diff.months()), | ||
}); | ||
if (diff.days()) | ||
return localize('Seen {{ duration }} day{{ prural }} ago', { | ||
duration: diff.days(), | ||
prural: addPrural(diff.days()), | ||
}); | ||
if (diff.hours()) | ||
return localize('Seen {{ duration }} hour{{ prural }} ago', { | ||
duration: diff.hours(), | ||
prural: addPrural(diff.hours()), | ||
}); | ||
if (diff.minutes()) | ||
return localize('Seen {{ duration }} minute{{ prural }} ago', { | ||
duration: diff.minutes(), | ||
prural: addPrural(diff.minutes()), | ||
}); | ||
} else { | ||
return localize('Seen more than 6 months ago'); | ||
} | ||
} | ||
return localize('Online'); | ||
}; | ||
|
||
return ( | ||
<Text color='less-prominent' size={size}> | ||
{last_online_label()} | ||
</Text> | ||
); | ||
}; | ||
|
||
OnlineStatusLabel.propTypes = { | ||
is_online: PropTypes.number.isRequired, | ||
last_online_time: PropTypes.number, | ||
size: PropTypes.string, | ||
}; | ||
|
||
export default observer(OnlineStatusLabel); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
.online-status { | ||
&__icon { | ||
width: 1rem; | ||
height: 1rem; | ||
margin: auto 0.5rem auto 0rem; | ||
border-radius: 50%; | ||
|
||
@include mobile { | ||
width: 0.8rem; | ||
height: 0.8rem; | ||
} | ||
|
||
&--online { | ||
background: var(--text-profit-success); | ||
} | ||
&--offline { | ||
background: var(--checkbox-disabled-grey); | ||
} | ||
} | ||
} |
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 see why we need this, the reason I suggested to show online is if it's your advert, at least it'll be visible online as compared to an incorrect offline status.
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 checked this, the stats are replaced by the previously viewed counterparty advertiser when you view your own adverts
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.
This includes the star rating as well
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 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 can add checklist to QA for this too
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 the issue is in the request, it's the id. If you can fix the id, you can use one request for advertiser page.
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.
So I found the cause of this issue:
this.advertiser_details_id
in the advertiser page store is correctly pointing to the clicked advertiser ID, but when we navigate to our own adverts, we make a call top2p_advertiser_info
withsubscribe: 1
and the id as our advertiser ID, but this returns an error from the websocket.This is because we are already subscribed to listen to our own advertiser details within general store, and thus the reason why the websocket is returning
AlreadySubscribed
error (by omitting theid
field, we are essentially calling advertiser info on our own profile as per the API documentation):So, one of the ways I tried to use a single request was modifying
this.counterparty_advertiser_info
into usinggeneral_store.advertiser_info
if we are looking at our own adverts, but this defeats the purpose of having a counterparty advertiser info as an observable, since it could then contain your own advertiser info or the counterparty advertiser info. Since we are already subscribed to our ownp2p_advertiser_info
endpoint, I feel that we can just reuse our details withingeneral_store.advertiser_info
without making unnecessary re-request to again get our personal advertiser info, since its already cached within general store. If there are any updates to our own advertiser info, the subscribed endpoint in general store should automatically already re-set our advertiser info throughgeneral_store.updateAdvertiserInfo
:Thus the approach I can find at the moment without breaking other pages using
counterparty_advertiser_info
is to just check within advertiser page, that is:general_store.advertiser_info
, any subsequent updates to our advertiser info will be updated automatically since we are already subscribed to it in general store (and it would not be unsubscribed even if we leave our advertiser page and check on other user's advertiser info)advertiser_page_store.counterparty_advertiser_info
advertiser_page_store.advertiser_details_id === general_store.advertiser_id
If this approach is not a good one, I could try to create a new computed value to return either
general_store.advertiser_info
oradvertiser_page_store.counterparty_advertiser_info
, but that is essentially the same approach as I am doing within advertiser pageThere 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 this is not acceptable, I can add a TODO and request to create a new card for this as enhancement, since at the moment this PR is urgent to be moved to Needs QA by today. I tried to modify some of the actions within advertiser page store to swap with
general_store.advertiser_info
, but it caused some issues where the page is not loading and the advertiser page is still showing the previous counterparty advertiser info T_T