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

Studio: Open the site's URL from the Sync connect popup #799

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open

Studio: Open the site's URL from the Sync connect popup #799

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c83edb4
Make links open in sync modal
Jan 10, 2025
fd4ee5a
Fix selected state
Jan 10, 2025
4ab21ae
Add arrow icon
Jan 15, 2025
cd840ad
Merge branch 'trunk' of github.com:Automattic/studio into fix/open-si…
Jan 15, 2025
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
15 changes: 12 additions & 3 deletions src/components/sync-sites-modal-selector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useState, useEffect } from 'react';
import { useOffline } from '../hooks/use-offline';
import { cx } from '../lib/cx';
import { getIpcApi } from '../lib/get-ipc-api';
import { ArrowIcon } from './arrow-icon';
import { Badge } from './badge';
import Button from './button';
import { CreateButton } from './connect-create-buttons';
Expand Down Expand Up @@ -206,11 +207,19 @@ function SiteItem( {
<div className={ cx( 'a8c-body truncate', ! isSyncable && 'text-a8c-gray-30' ) }>
{ site.name }
</div>
<div
className={ cx( 'a8c-body-small text-a8c-gray-30 truncate', isSelected && 'text-white' ) }
<Button
variant="link"
className={ cx(
'a8c-body-small truncate !p-0',
isSelected
? '!text-inherit hover:!text-inherit'
: '!text-a8c-gray-30 hover:!text-a8c-gray-30'
) }
onClick={ () => getIpcApi().openURL( site.url ) }
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 did not add this arrow but perhaps we can consider adding it to indicate to the user that it is a link:

Screenshot 2025-01-10 at 2 20 03 PM

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.

Yes, we should stick to this convention to provide consistent UX.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I made updates in 4ab21ae

Let me know what 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.

Looks great, thanks.

>
{ site.url.replace( /^https?:\/\//, '' ) }
</div>
<ArrowIcon />
</Button>
</div>
{ isSyncable && (
<div className="flex gap-2">
Expand Down
Loading