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

feat: confirmation dialog for deeplinks #783

Merged
merged 6 commits into from
Jan 26, 2025

Conversation

lily-de
Copy link
Collaborator

@lily-de lily-de commented Jan 26, 2025

Screenshot 2025-01-25 at 10 09 03 PM Screenshot 2025-01-25 at 10 11 01 PM

@lily-de lily-de changed the title Ldelalande/confirmation dialog for deeplinks feat: confirmation dialog for deeplinks Jan 26, 2025
Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

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

Pre-stamping, but check out the comments and see what you think!

import 'react-toastify/dist/ReactToastify.css';
import { ToastContainer } from 'react-toastify';
import { ModelProvider } from './components/settings/models/ModelContext';
import { ActiveKeysProvider } from './components/settings/api_keys/ActiveKeysContext';

export default function App() {
const [fatalError, setFatalError] = useState<string | null>(null);
const [modalVisible, setModalVisible] = useState(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we use a more descriptive name of which modal? Something like confirmExtensionModalVisible

}) {
return (
<div className="fixed inset-0 bg-black/20 backdrop-blur-sm z-[9999]">
<div className="fixed top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 w-[400px] bg-white dark:bg-gray-800 rounded-xl shadow-xl border border-gray-200 dark:border-gray-700">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did we reference other modals when grabbing all the bg- border- etc classes for this?

We might compare backgrounds to the ones available from @nahiyankhan's big stying push now that we have nice things like bg-subtle

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i copied what you have in the extensions folder but will double check!

@michaelneale
Copy link
Collaborator

it is also possible to use electron.show from main.ts if it was less trouble (fair bit of work to weave in a modal from the renderer side) - just one option (but this gives more control of course)

@baxen baxen force-pushed the ldelalande/confirmation-dialog-for-deeplinks branch from ba7808e to bbb69b3 Compare January 26, 2025 07:55
@lily-de lily-de merged commit 4548710 into main Jan 26, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants