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

290 desktop client should not force user to setup a local core #324

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

jhuang38
Copy link
Contributor

@jhuang38 jhuang38 commented Aug 1, 2023

Description

Added way for users to skip local core setup in initial page load in Tauri:
image

Added extra button in core select page in Tauri for local setup later on - this works by selecting the first option that shows up in Tauri, which might not work in all cases.
image

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Note: make sure your files are formatted with rust-analyzer

@jhuang38 jhuang38 requested a review from Ynng August 1, 2023 05:10
@jhuang38 jhuang38 linked an issue Aug 1, 2023 that may be closed by this pull request
@netlify
Copy link

netlify bot commented Aug 1, 2023

Deploy Preview for lodestone-dashboard ready!

Name Link
🔨 Latest commit bb7aad3
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-dashboard/deploys/64e94b41880f4200088395b5
😎 Deploy Preview https://deploy-preview-324--lodestone-dashboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 1, 2023

Deploy Preview for lodestone-storybook ready!

Name Link
🔨 Latest commit bb7aad3
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-storybook/deploys/64e94b41b3584c000740069c
😎 Deploy Preview https://deploy-preview-324--lodestone-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@jhuang38 jhuang38 changed the base branch from main to dev August 1, 2023 05:30
@jhuang38 jhuang38 requested a review from Ynng August 2, 2023 01:26
@jhuang38 jhuang38 self-assigned this Aug 2, 2023
@Ynng
Copy link
Member

Ynng commented Aug 10, 2023

We need to disable this popup in this usecase (tauri & localcore not setup & on dashboard page looking at external cores)

image

@Ynng
Copy link
Member

Ynng commented Aug 10, 2023

This page might benefit from a "back" button

image

@Ynng
Copy link
Member

Ynng commented Aug 10, 2023

This button still says "setup" even if the local core is already setup.

We should remove this button, or change the text to be more general like "use local core".

image

@jhuang38
Copy link
Contributor Author

This page might benefit from a "back" button

image

This page might benefit from a "back" button

image

which page should we go back to? in this issue there are 2 ways of going to that page, from this one:
image

and this one:
image

@jhuang38
Copy link
Contributor Author

We need to disable this popup in this usecase (tauri & localcore not setup & on dashboard page looking at external cores)

image

could you clarify on the exact steps you did to get this? i can't seem to get to this popup

@Ynng
Copy link
Member

Ynng commented Aug 19, 2023

Regarding back button, I think the default browser "navigate backward" behavior might be enough.
For the "new local core detected" prompt, reproduce it like this:

  1. setup lodestone core on a separate machine
  2. delete your local lodestone core
  3. open lodestone desktop, click connect to existing core
  4. enter details and get to the dashboard
  5. it should show up

type="button"
iconRight={faArrowLeft}
label="Back"
onClick={navigateBack}
/> */}
onClick={() => {
Copy link
Member

Choose a reason for hiding this comment

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

can this be shortened to onClick={navigateBack}?

return;
}

// if (coreList.length > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

why is this redirect commented out?

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.

Desktop client should not force user to setup a local core
2 participants