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

fix issue 132 #191

Merged
merged 9 commits into from
Aug 6, 2024
Merged

fix issue 132 #191

merged 9 commits into from
Aug 6, 2024

Conversation

Guocork
Copy link
Collaborator

@Guocork Guocork commented Aug 3, 2024

No description provided.

@Guocork Guocork closed this Aug 3, 2024
@Guocork Guocork reopened this Aug 3, 2024
@Guocork Guocork marked this pull request as draft August 3, 2024 04:04
@Guocork Guocork marked this pull request as ready for review August 3, 2024 04:36
@Guocork Guocork marked this pull request as draft August 3, 2024 04:36
@Guocork Guocork marked this pull request as ready for review August 5, 2024 09:13
@jmbejar jmbejar changed the base branch from main to dev August 5, 2024 19:08
Copy link
Collaborator

@jmbejar jmbejar left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this issue. The code looks good 👏 👏
However, I found an issue to fix before merging it. Please let me know if you need extra help or information to wrap up the PR.

@@ -134,7 +134,9 @@ impl Widget for ModelSelectorList {
cx.begin_turtle(walk, self.layout);

if self.visible {
self.draw_items(cx, &store.downloads.downloaded_files);
if let Some(loaded_file) = store.get_loaded_downloaded_file() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is an issue. It could happen that there is no loaded_file, for this reason is an Option value.

You can replicate this corner case if you go to "My Models" and remove the current active model, and go back to the chat screen:

image

Looks like you can send the result of store.get_loaded_downloaded_file() and attempt to unwrap it in the draw_items function.

Copy link
Collaborator

@jmbejar jmbejar left a comment

Choose a reason for hiding this comment

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

I found a minor issue with a loading bar in the model selector, but let's get this merged. I can fix it in a separate commit.

Thanks! 🎉

@jmbejar jmbejar merged commit f906fac into moxin-org:dev Aug 6, 2024
5 checks passed
@Guocork
Copy link
Collaborator Author

Guocork commented Aug 7, 2024

Anyway,I also found a situation where the arrow icon shouldn't appear when there are no available models, like this.
截图 2024-08-07 10-12-55

@Guocork
Copy link
Collaborator Author

Guocork commented Aug 8, 2024

@jmbejar ,I think the arrow icon shouldn't appear when there are no available models, like this.
image
I don't know if this situation is the minor issue you mentioned.

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.

2 participants