Skip to content

feat: refactor output box #93

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

Merged
merged 2 commits into from
Nov 25, 2022
Merged

feat: refactor output box #93

merged 2 commits into from
Nov 25, 2022

Conversation

nibilin33
Copy link
Contributor

@nibilin33 nibilin33 commented Nov 25, 2022

issue 74

  1. move toggle button to outputbox
  2. fix dashboard loading

3E6158D1-624A-42BC-B166-79E54B59A262

1. move toggle button to outputbox
2. fix dashboard loading
Copy link
Collaborator

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

Thanks! The two things in this PR do not seem to be related. I would suggest handling only issue #71 in this PR and leaving the second "dashboard loading fix" to another PR. Also, what was the dashboard loading issue?

Comment on lines 299 to 308
<Repos
callback={(arg) => {
if (arg) {
setError(arg);
}
}}
onLoading={(value) => {
setLoading(value);
}}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What was the issue here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no issue, i just find that there are two loading hint. so i fixed it in the same time

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, let’s remove this, because this adds extra complexity. We’ll redirect to login page anyway, so there won’t be warnings. #77

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 remove error part, remain the loading part

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worry. I'm taking over this part. But I want to confirm if it redirects the login page immediately or gives any hints and then jump to the login page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

normally, it needs show hint firstly and then redirects

@lihebi
Copy link
Collaborator

lihebi commented Nov 25, 2022

And your image link seems to be broken?

Copy link
Collaborator

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

Thanks!

@lihebi lihebi merged commit 1b12292 into codepod-io:main Nov 25, 2022
Comment on lines +295 to +298
<Repos
onLoading={(value)=>{
setLoading(value)
}}/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI #106

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