Skip to content

@mlodyjesienin/example app UI #386

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

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

mlodyjesienin
Copy link

@mlodyjesienin mlodyjesienin commented Jun 10, 2025

Description

Migrated example apps (llm, computervision) to expo router navigation.
Added drawer navigation and menu for better UX.
Fixed small issues regarding keyboard padding.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Tested on

  • iOS
  • Android

Related issues

Issue #224

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas, (not necessary)
  • I have updated the documentation accordingly, (not necessary)
  • My changes generate no new warnings

<Drawer.Screen
name="index"
options={{
drawerLabel: ' Menu',
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 a space here?

"react-native-safe-area-context": "5.4.0",
"react-native-screens": "~4.11.1",
"react-native-svg": "15.11.2",
"react-native-svg-transformer": "^1.5.0",
"react-native-wheel-scrollview-picker": "^2.0.6"
Copy link
Member

@msluszniak msluszniak Jun 10, 2025

Choose a reason for hiding this comment

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

I don't understand how this PR tackles #224 if scrollview-picker is still in package.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, remove scrollview-picker from each package.json and reinstall deps.

@msluszniak
Copy link
Member

Calling yarn in example apps generates now new warnings:

➤ YN0002: │ computer-vision@workspace:apps/computer-vision doesn't provide @react-navigation/native (p65dbd), requested by @react-navigation/drawer.
➤ YN0002: │ llm@workspace:apps/llm doesn't provide @react-navigation/native (pe0aa0), requested by @react-navigation/drawer.

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

Could you also handle this one: #349 in this PR and disable voice chat on emulator in llm examples?

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

This seems to be a bit of redudancy:
image

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

If I'm not wrong, currently tool calling supports only calls to Google Calendar, not the regular calendar on device. So make it clear and add Google Calendar instead of calendar.
My bad, ignore this comment. This not work for me on emulator, so I'll create separate issue about this.
image

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

I guess that the icon in the chat presents a lama because the model was previously LLaMa. Now it's Qwen, so maybe think of changing this one.
image

@msluszniak
Copy link
Member

I think that in llm examples it will be better to use simple stack navigation with back arrow to the main screen instead of drawer navigation.

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

Could you disable the button with camera on emulator for all examples in apps/computer-vision?
image
PS. change order of words to Top 10 Results, sounds better imho.

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

I don't understand what should present the Object Detection example in apps/computer-vision. Does it even work?

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

The fact that OCR and Vertical OCR on the list below are not two consecutive elements is unnatural for me. Maybe consider changing the name to OCR Vertical to preserve alphabetical order and force the intuitive order.
image

@msluszniak
Copy link
Member

Maybe it is worth adding upload from device option for speech to text example?

@msluszniak
Copy link
Member

There is no way now to clear automatically added sentences and user defined sentences. I think it will be nice to add such option.
image

@msluszniak
Copy link
Member

msluszniak commented Jun 10, 2025

Deactivate Find Similar when no input provided, and communicate more explicitly in this example that you need to provide sentence and use Find Similar on that particular sentence. I thought, that I should enter all context sentences by Add to list and then clicking Find Similar will generate me some text embeddings similar to the context sentences.

@msluszniak
Copy link
Member

In computer-vision package.json there is "react-native-executorch": "0.4.2", while other examples use react-native-executorch from monorepo, maybe change this one.

@@ -7,6 +7,7 @@
"icon": "./assets/icons/icon.png",
"userInterfaceStyle": "light",
"newArchEnabled": true,
"scheme": "your-app-scheme",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to set scheme to "rne-computer-vision"

Do the same for other apps

"expo-status-bar": "~2.2.3",
"metro-config": "^0.81.0",
"react": "19.0.0",
"react-native": "0.79.2",
"react-native-executorch": "workspace:*",
"react-native-executorch": "0.4.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"react-native-executorch": "0.4.2",
"react-native-executorch": "workspace:*",

@@ -6,6 +6,7 @@
"orientation": "portrait",
"icon": "./assets/icons/icon.png",
"userInterfaceStyle": "light",
"scheme": "your-app-scheme",
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as in computer vision rne-llm

@@ -42,10 +38,6 @@ export default function LLMToolCallingScreen({
tokenizerConfigSource: HAMMER2_1_TOKENIZER_CONFIG,
});

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you ensured that it's impossible to change the route when model is generating, because if it's possible it will lead to errors when trying to load model on a different page. I think with current implementation it isn't handled.

@@ -69,7 +65,7 @@ export default function VoiceChatScreen({
tokenizerConfigSource: QWEN3_TOKENIZER_CONFIG,
});
const speechToText = useSpeechToText({
modelName: 'moonshine',
modelName: 'whisper',
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change? I think w should stay with moonshine as default

"react-native-safe-area-context": "5.4.0",
"react-native-screens": "~4.11.1",
"react-native-svg": "15.11.2",
"react-native-svg-transformer": "^1.5.0",
"react-native-wheel-scrollview-picker": "^2.0.6"
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, remove scrollview-picker from each package.json and reinstall deps.

@mlodyjesienin mlodyjesienin marked this pull request as draft June 12, 2025 11:40
<action android:name="android.intent.action.VIEW"/>
<category android:name="android.intent.category.DEFAULT"/>
<category android:name="android.intent.category.BROWSABLE"/>
<data android:scheme="your-app-scheme"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to run npx expo prebuild for a changes from app.json to be applied, but when doing that don't forget to open ios project in xcode and add increased memory capability

return context;
}

import { ReactNode } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the import to the top of this file

@NorbertKlockiewicz
Copy link
Contributor

I've tested it and works fine. Please apply changes requested i my two comments, fix the conflicts and you can merge.

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.

Find alternative for scroll picker with better UX for example apps.
3 participants