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

AK-14 - Added page titles #306

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions src/features/accounts/pages/account-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { AccountDetails } from '../components/account-details'
import { useCallback } from 'react'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useTitle } from '@/utils/use-title'

export const accountPageTitle = 'Account'
export const accountInvalidAddressMessage = 'Address is invalid'
Expand All @@ -29,6 +30,7 @@ export function AccountPage() {
const { address } = useRequiredParam(UrlParams.Address)
invariant(isAddress(address), accountInvalidAddressMessage)
const [loadableAccount, refreshAccount, isStale] = useLoadableAccount(address)
useTitle()

const refresh = useCallback(() => {
refreshAccount()
Expand Down
2 changes: 2 additions & 0 deletions src/features/app-interfaces/pages/create-app-interface.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useCreateAppInterface } from '../data'
import { FromAppIdCard } from '../components/create/from-app-id-card'
import { FromDeploymentCard } from '../components/create/from-deployment-card'
import { asError } from '@/utils/error'
import { useTitle } from '@/utils/use-title'

export const createAppInterfacePageTitle = 'Create App Interface'

Expand Down Expand Up @@ -103,6 +104,7 @@ function CreateAppInterfaceInner() {
}

export function CreateAppInterface() {
useTitle('Create App Interface')
return (
<>
<PageTitle title={createAppInterfacePageTitle} />
Expand Down
2 changes: 2 additions & 0 deletions src/features/app-lab/pages/app-lab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { useAppInterfaces } from '@/features/app-interfaces/data'
import { RenderLoadable } from '@/features/common/components/render-loadable'
import { PageLoader } from '@/features/common/components/page-loader'
import { AppInterfaces } from '@/features/app-interfaces/components/app-interfaces'
import { useTitle } from '@/utils/use-title'

export const appLabPageTitle = 'App Lab'

export function AppLab() {
const [appInterfaces, refreshAppInterfaces] = useAppInterfaces()
useTitle('App Lab')

return (
<>
Expand Down
2 changes: 2 additions & 0 deletions src/features/applications/pages/application-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { is404 } from '@/utils/error'
import { useCallback } from 'react'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useTitle } from '@/utils/use-title'

const transformError = (e: Error) => {
if (is404(e)) {
Expand All @@ -31,6 +32,7 @@ export function ApplicationPage() {

const applicationId = parseInt(_applicationId, 10)
const [loadableApplication, refreshApplication, isStale] = useLoadableApplication(applicationId)
useTitle()

const refresh = useCallback(() => {
refreshApplication()
Expand Down
2 changes: 2 additions & 0 deletions src/features/assets/pages/asset-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useLoadableAsset } from '../data'
import { useCallback } from 'react'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useTitle } from '@/utils/use-title'

const transformError = (e: Error) => {
if (is404(e)) {
Expand All @@ -31,6 +32,7 @@ export function AssetPage() {

const assetId = parseInt(_assetId, 10)
const [loadableAsset, refreshAsset, isStale] = useLoadableAsset(assetId)
useTitle()

const refresh = useCallback(() => {
refreshAsset()
Expand Down
2 changes: 2 additions & 0 deletions src/features/blocks/pages/block-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useLoadableBlock } from '../data'
import { isInteger } from '@/utils/is-integer'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useTitle } from '@/utils/use-title'

const transformError = (e: Error) => {
if (is404(e)) {
Expand All @@ -25,6 +26,7 @@ export const blockInvalidRoundMessage = 'Round is invalid'
export const blockFailedToLoadMessage = 'Block failed to load'

export function BlockPage() {
useTitle()
const { round: _round } = useRequiredParam(UrlParams.Round)
invariant(isInteger(_round), blockInvalidRoundMessage)
const round = parseInt(_round, 10)
Expand Down
2 changes: 2 additions & 0 deletions src/features/explore/pages/explore-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import { LatestTransactions } from '@/features/transactions/components/latest-tr
import { Switch } from '@/features/common/components/switch'
import { Label } from '@/features/common/components/label'
import { useLiveExplorer } from '@/features/explore/data/live-explorer'
import { useTitle } from '@/utils/use-title'

export const explorePageTitle = 'Explore'

export function ExplorePage() {
const { showLiveUpdates, setShowLiveUpdates, latestTransactions, latestBlocks } = useLiveExplorer()
useTitle('Explore')

return (
<>
Expand Down
2 changes: 2 additions & 0 deletions src/features/fund/fund-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ import { PageTitle } from '@/features/common/components/page-title'
import { localnetId, useNetworkConfig } from '@/features/network/data'
import { LocalnetFunding } from './components/localnet-funding'
import { DispenserApiFunding } from './components/dispenser-api-funding'
import { useTitle } from '@/utils/use-title'

export const fundPageTitle = 'Fund'
export const fundingNotAvailableMessage = 'Funding is not available on this network.'

export function FundPage() {
const networkConfig = useNetworkConfig()
useTitle('Fund')

const inner =
networkConfig.id === localnetId ? (
Expand Down
2 changes: 2 additions & 0 deletions src/features/groups/pages/group-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { invariant } from '@/utils/invariant'
import { isInteger } from '@/utils/is-integer'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useTitle } from '@/utils/use-title'

export const groupPageTitle = 'Transaction Group'
export const groupNotFoundMessage = 'Transaction group not found'
Expand All @@ -28,6 +29,7 @@ export function GroupPage() {
const { round: _round } = useRequiredParam(UrlParams.Round)
invariant(isInteger(_round), blockInvalidRoundMessage)
const { groupId } = useRequiredParam(UrlParams.GroupId)
useTitle()

const round = parseInt(_round, 10)
const loadableGroup = useLoadableGroup(groupId, round)
Expand Down
2 changes: 2 additions & 0 deletions src/features/settings/pages/settings-page.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { useTitle } from '@/utils/use-title'
import { Settings } from '../components/settings'
import { PageTitle } from '@/features/common/components/page-title'

export const settingsPageTitle = 'Settings'

export function SettingsPage() {
useTitle('Settings')
return (
<>
<PageTitle title={settingsPageTitle} />
Expand Down
2 changes: 2 additions & 0 deletions src/features/transaction-wizard/transaction-wizard-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import { SendTransactionResults } from '@algorandfoundation/algokit-utils/types/
import { AppCallTransaction, TransactionType } from '../transactions/models'
import { GroupSendResults, SendResults } from './components/group-send-results'
import algosdk from 'algosdk'
import { useTitle } from '@/utils/use-title'

export const transactionWizardPageTitle = 'Transaction Wizard'
export const transactionTypeLabel = 'Transaction type'
export const sendButtonLabel = 'Send'

export function TransactionWizardPage() {
const [sendResults, setSendResults] = useState<SendResults | undefined>(undefined)
useTitle('Transaction Wizard')

const renderTransactionResults = useCallback((result: SendTransactionResults, simulateResponse?: algosdk.modelsv2.SimulateResponse) => {
const sentTransactions = asTransactionFromSendResult(result)
Expand Down
2 changes: 2 additions & 0 deletions src/features/transactions/pages/inner-transaction-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { is404 } from '@/utils/error'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useSplatParam } from '@/features/common/hooks/use-splat-param'
import { useTitle } from '@/utils/use-title'

const transformError = (e: Error) => {
if (is404(e)) {
Expand All @@ -34,6 +35,7 @@ export function InnerTransactionPage() {
invariant(isValidInnerTransactionId(innerTransactionId), `Invalid inner transaction id: ${innerTransactionId}`)

const loadableTransaction = useLoadableInnerTransactionAtom(transactionId, innerTransactionId)
useTitle()

return (
<>
Expand Down
2 changes: 2 additions & 0 deletions src/features/transactions/pages/transaction-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useLoadableTransactionAtom } from '../data'
import { isTransactionId } from '@/utils/is-transaction-id'
import { PageTitle } from '@/features/common/components/page-title'
import { PageLoader } from '@/features/common/components/page-loader'
import { useTitle } from '@/utils/use-title'

const transformError = (e: Error) => {
if (is404(e)) {
Expand All @@ -28,6 +29,7 @@ export function TransactionPage() {
const { transactionId } = useRequiredParam(UrlParams.TransactionId)
invariant(isTransactionId(transactionId), transactionInvalidIdMessage)
const loadableTransaction = useLoadableTransactionAtom(transactionId)
useTitle()

return (
<>
Expand Down
47 changes: 47 additions & 0 deletions src/utils/use-title.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { useEffect } from 'react'
import { useParams } from 'react-router-dom'

const setTitle = (title: string) => {
document.title = title
}

export const useTitle = (customString?: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const useTitle = (customString?: string) => {
export const useTitle = (pagePrefix?: string) => {

My thought process here is that anything after the Lora - is the page specific portion of the title, so it's a page prefix, even though it's not the title prefix.

const urlParams = useParams()
useEffect(() => {
const currentTitle = document.title
let pageTitle = `Lora`
Copy link
Contributor

@neilcampbell neilcampbell Oct 30, 2024

Choose a reason for hiding this comment

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

I think it would be good for all pages to start with Lora - ${pageTitle}
So:
Lora - Transaction Wizard
Lora - TxnId:CFNKY4JV5BMIRSP2SMZE634DYB22FALLURI72U6UMWZNKEBOYEMQ - mainnet

Probably the easiest way to do this is accumulate in an array, then .join(' - ') to build the page title. Something like below:

  const params = new Array<string>()

  if (urlParams.transactionId) {
    params.push(`TxnId:${urlParams.transactionId}`)
  }
  // ...

  const title = `Lora${pagePrefix ? ` - ${pagePrefix}` : ''} - ${params.join(' - ')}`

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a small preference for omitting the Id part in transactions / assets / apps

So: Txn:${id} App:${id} Asset:${id}

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

And if we omit the - separators, browser tabs would show more of the title before truncating:

Lora Txn ACSASDSADASD Mainnet vs Lora - Txn:ACSASDSADASD - Mainnet

May not make much difference though, so this is quite subjective

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happy with both of those suggestions.

Copy link
Author

Choose a reason for hiding this comment

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

Me too. I'll make these updates today.

if (customString) {
pageTitle += ` ${customString}`
}
if (urlParams.transactionId) {

Check failure on line 16 in src/utils/use-title.ts

View workflow job for this annotation

GitHub Actions / Run PR checks / node-ci

src/features/fund/fund-page.test.tsx > fund-page > when on localnet > should render the localnet funding controls

TypeError: Cannot read properties of undefined (reading 'transactionId') ❯ src/utils/use-title.ts:16:19 ❯ commitHookEffectListMount node_modules/react-dom/cjs/react-dom.development.js:23150:26 ❯ commitPassiveMountOnFiber node_modules/react-dom/cjs/react-dom.development.js:24931:11 ❯ commitPassiveMountEffects_complete node_modules/react-dom/cjs/react-dom.development.js:24891:9 ❯ commitPassiveMountEffects_begin node_modules/react-dom/cjs/react-dom.development.js:24878:7 ❯ commitPassiveMountEffects node_modules/react-dom/cjs/react-dom.development.js:24866:3 ❯ flushPassiveEffectsImpl node_modules/react-dom/cjs/react-dom.development.js:27039:3 ❯ flushPassiveEffects node_modules/react-dom/cjs/react-dom.development.js:26984:14 ❯ node_modules/react-dom/cjs/react-dom.development.js:26769:9 ❯ flushActQueue node_modules/react/cjs/react.development.js:2667:24
pageTitle += ` - TxnId:${urlParams.transactionId}`
}
if (urlParams.transactionId && urlParams['*']) {
pageTitle += `, Inner:${urlParams['*']}`
}
if (urlParams.round) {
pageTitle += ` - Block:${urlParams.round}`
}
if (urlParams?.groupId) {
pageTitle += ` - Group:${urlParams.groupId}`
}
if (urlParams?.address) {
pageTitle += ` - Addr:${urlParams.address}`
Copy link
Contributor

@tasosbit tasosbit Oct 30, 2024

Choose a reason for hiding this comment

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

The verbiage we use in these pages is Account (e.g. check <H1> there), so the document title should probably be Account or Acct instead of Addr to match that

Copy link
Author

Choose a reason for hiding this comment

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

👍 Will change to use Acct

}
if (urlParams?.applicationId) {
pageTitle += ` - AppId:${urlParams.applicationId}`
}
if (urlParams?.assetId) {
pageTitle += ` - AssetId:${urlParams.assetId}`
}
if (urlParams?.networkId) {
pageTitle += ` - ${urlParams.networkId}`
}

setTitle(pageTitle)

return () => {
setTitle(currentTitle)
}
}, [customString, urlParams])
}
Loading