Conversation
knownasred
left a comment
knownasred
left a comment
There was a problem hiding this comment.
Some usability remarks
| "https://192.168.20.88:3000/", | ||
| "https://192.168.20.88:3000" |
There was a problem hiding this comment.
This is flakey, as IPs are not fixed depending on the network. I would rather allow all origins instead
| <!-- Mobile navigation and content --> | ||
| <MobileBottomNavbar /> | ||
| <MobileContentContainer /> |
There was a problem hiding this comment.
Is there a way to wrap this inside of a block md:hidden instead, and remove those directives from the Mobile Bottom and content, so that the logic for display is in the same place for the two?
| import { defineConfig } from 'vitest/config'; | ||
| import glsl from 'vite-plugin-glsl'; | ||
| import process from 'node:process'; | ||
| import mkcert from 'vite-plugin-mkcert'; |
There was a problem hiding this comment.
can you modify the readme to mention that mkcert is used? There needs to be a bit of additional setup in browsers for people that wants to work with the code
87b97e7 to
4038bb0
Compare
4038bb0 to
60e63b6
Compare
60e63b6 to
0f5fdf6
Compare
e587c7e to
0bec301
Compare
0f5fdf6 to
a958d6b
Compare
0bec301 to
6aa25d7
Compare
9b1b219 to
1c55f7f
Compare
6aa25d7 to
74ae769
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
d14e70f to
7a6f9ff
Compare
6d1d380 to
e74a4d3
Compare
e74a4d3 to
be8d1a8
Compare
| import mkcert from 'vite-plugin-mkcert'; | ||
|
|
||
| export default defineConfig({ |
There was a problem hiding this comment.
The mkcert plugin is imported but not added to the plugins array on line 8. This means HTTPS certificates will not be generated, breaking the HTTPS functionality that the version bump in bun.lock suggests is needed.
Fix by adding mkcert to the plugins:
plugins: [glsl(), sveltekit(), wasm(), mkcert()],Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Deployment informationsThis PR has passed automatic testing, and is ready to be tested manually. Warning You might need to have a logged-in employee runelabs account to access the deployment environment.
This comment was written by a bot! |
| // Skip animation updates if disabled or on mobile | ||
| if (!enableAnimation && !deviceStore.isMobile) return; |
There was a problem hiding this comment.
Logic error: The condition !enableAnimation && !deviceStore.isMobile is backwards. The comment says "Skip animation updates if disabled or on mobile" but the code skips only when animation is disabled AND not on mobile. This will cause animations to continue running on mobile devices, defeating the performance optimization.
// Fix: Should be OR instead of AND
if (!enableAnimation || deviceStore.isMobile) return;| // Skip animation updates if disabled or on mobile | |
| if (!enableAnimation && !deviceStore.isMobile) return; | |
| // Skip animation updates if disabled or on mobile | |
| if (!enableAnimation || deviceStore.isMobile) return; |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
b04908a to
2b32b58
Compare
| const usdcAddress = | ||
| '0x033068f6539f8e6e6b131e6b2b814e6c34a5224bc66947c47dab9dfee93b35fb'; | ||
| return data.availableTokens.find((token) => token.address === usdcAddress); | ||
| return data.availableTokens.find((token) => token.address === usdcBridgedAddress); |
There was a problem hiding this comment.
This line causes CI to fail because it doesn't conform to Prettier's formatting standards. Run 'bun prettier --write .' to fix the formatting.
Spotted by Graphite Agent (based on CI logs)
Is this helpful? React 👍 or 👎 to let us know.
2 participants
feat: navbar on mobile
change touch settings