Skip to content

Conversation

@pandablue0809 pandablue0809 requested a review from ihomp September 29, 2025 09:45
@pandablue0809
Copy link
Member Author

@ihomp
i fixed it. plz review again. 🙏

if (!option) return
if (option.username && !option.username.includes('-')) {
onSearch(option.username)
onSearch(option.username, option?.tag)
Copy link
Member

Choose a reason for hiding this comment

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

you don't need "?", as on line 155 there is a check that it exists

Copy link
Member

Choose a reason for hiding this comment

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

thats why option.username doesn't have the "?"

onSearch(option.username, option?.tag)
} else {
onSearch(option.address)
onSearch(option.address, option?.tag)
Copy link
Member

Choose a reason for hiding this comment

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

same here

}

if (isValidPayString(searchFor) || isValidXAddress(searchFor)) {
if (isValidPayString(searchItem) || isValidXAddress(searchItem)) {
Copy link
Member

Choose a reason for hiding this comment

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

why did you change searchFor to searchItem ?

I can see on lines 287 and lower it is still searchFor

// if there is a tag -
// get the new page which we can show an address and a tag
router.push('/account/' + encodeURI(searchFor) + addParams) //replace with a new page to show a tag
if(tag) {
Copy link
Member

Choose a reason for hiding this comment

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

it seems no prettify used, it should be a space between if and (tag). please use prettify before sending the PR

// Fetch account data for the resolved address
const accountResponse = await axiosServer({
method: 'get',
url: `v2/address/${address}?username=true&service=true&verifiedDomain=true&parent=true&nickname=true&inception=true&flare=true&blacklist=true&payString=true&ledgerInfo=true&xamanMeta=true&bithomp=true&obligations=true`,
Copy link
Member

Choose a reason for hiding this comment

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

do you really need all those flags here?
do you show all this info?

<>
<SEO
page="Account with Tag"
title={`${t('explorer.header.account')} ${userData.service || userData.username || userData.address} - Tag: ${destinationTag}`}
Copy link
Member

Choose a reason for hiding this comment

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

"X-Address information"

Copy link
Member

@ihomp ihomp left a comment

Choose a reason for hiding this comment

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

  • PayString is not resolved
    /account/username$paystring.crypto.com

  • x-Address opens an account page without the tag
    /account/XVVFXHFdehYhofb7XRWeJYV6kjTEwbq2mLScCiYyDTHKu9E

  • Direct searching for paystring / xAddress in the landing search - doesn't end up in the right page. (it works if chose from the dropdown)

  • we don't need /account/tag route; we need to reuse the /account/

  • the /account/tag should become a component like Xaddress

  • css need an update, it added some new styles - that are not common in our design.

  • also the social icons shouldn't be grey - it looks like they are disabled/not active

@pandablue0809
Copy link
Member Author

@ihomp
i fixed it.
and i use searchItem because searchFor is always r-address.
so i can't check x-address or paystring using this.
plz review again. 🙏

@pandablue0809 pandablue0809 requested a review from ihomp November 25, 2025 13:45
import { serverSideTranslations } from 'next-i18next/serverSideTranslations'
import Image from 'next/image'
import { axiosServer, getFiatRateServer, passHeaders } from '../../utils/axios'
import { isValidXAddress } from 'ripple-address-codec'
Copy link
Member

Choose a reason for hiding this comment

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

it's better to import function to check if X-address Valid from our file '../../utils'

in case we will update it later, so, it will be updated in the whole app at once.

pages/_app.js Outdated
import '../styles/globals.css'
import '../styles/ui.scss'
import '../styles/components/nprogress.css'
import '../styles/pages/xaddress-details.scss'
Copy link
Member

Choose a reason for hiding this comment

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

we should not load css specific for xaddress page to the whole app.
that css should be loaded on the component level.

Copy link
Member

@ihomp ihomp left a comment

Choose a reason for hiding this comment

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

css file in a wrong dir and should be loaded on the component lvl

@ihomp
Copy link
Member

ihomp commented Dec 4, 2025

The css file should be in the same sub dir as the component it used for

styles/components/Account/xaddress-details.scss

it should not be loadded in the app.js, it should be loaded in the component xaddress-details.js

@pandablue0809
Copy link
Member Author

@ihomp
i fixed it.
plz review again. 🙏

@pandablue0809 pandablue0809 requested a review from ihomp December 4, 2025 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants