Fixed existing issues and local testing problems#12
Open
AlphaTechini wants to merge 4 commits into
Open
Conversation
…ows. You should keep these changes as they enable the project to run correctly on your environment: Space in Folder Path The Problem: I put the repo in folder that had space in its name and the test config wasn't handling the space in the folder name correctly, causing the test server to fail. The Fix: I wrapped the path in quotes in playwright.config.ts. Even if you rename the folder later, keeping this is best practice as it prevents the error from happening again for anyone else with spaces in their path. Missing Authentication Logic The Problem: The logic to actually log a user in via a URL token was missing from the app, causing the auth.spec.ts test to fail. The Fix: I updated useAuth.ts to check the URL for a token and log the user in if one is present. This is a functional fix for a missing feature, not just a test patch. Windows Incompatibility The Problem: The build script tried to run chmod, a Linux/Mac command that doesn't exist on Windows, causing the build to crash. The Fix: I've updated the build script to be cross-platform. Instead of removing chmod entirely (which hurts Mac/Linux users), I replaced it with a small Node.js script that checks process.platform. It now runs chmod +x only if you are NOT on Windows" Then finally the best parts: The issues have been addressed. except the Claude market place. 1. Light theme available via moon/sun icon. 2. you can now view diff in split mode. 3. A banner should show when a new version is available on npm. Couldn't test this cos am on the latest version. Apologies for the long commit message
Author
Screen.Recording.2026-02-03.135144.mp4screen recording of implemented changes |
mcollina
reviewed
Feb 3, 2026
Owner
mcollina
left a comment
There was a problem hiding this comment.
Can you add some tests for these?
| @@ -0,0 +1,87 @@ | |||
| import { useState, useEffect } from 'react' | |||
|
|
|||
| const CURRENT_VERSION = '0.5.1' | |||
| hasUpdate: boolean; | ||
| } | ||
|
|
||
| function compareVersions (current: string, latest: string): boolean { |
Owner
There was a problem hiding this comment.
Can we use the semver module instead?
Author
There was a problem hiding this comment.
I'll update the version banner to use the semver module and read the version from
package.json instead. Then add tests.
…lly imported from package.json. Added tests for different scenarios.
Author
|
@mcollina sorry for the stress this should be okay now right? |
Owner
|
Can you add a test for the side-by-side view? |
Author
Done check my last commit |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
"I ran into some problems during local testing and fixed them as follows. You should keep these changes as they enable the project to run correctly on your environment:
Space in Folder Path
The Problem: I put the repo in folder that had space in its name and the test config wasn't handling the space in the folder name correctly, causing the test server to fail. The Fix: I wrapped the path in quotes in playwright.config.ts. Even if you rename the folder later, keeping this is best practice as it prevents the error from happening again for anyone else with spaces in their path. Missing Authentication Logic
The Problem: The logic to actually log a user in via a URL token was missing from the app, causing the auth.spec.ts test to fail. The Fix: I updated useAuth.ts to check the URL for a token and log the user in if one is present. This is a functional fix for a missing feature, not just a test patch. Windows Incompatibility
The Problem: The build script tried to run chmod, a Linux/Mac command that doesn't exist on Windows, causing the build to crash. The Fix: I've updated the build script to be cross-platform. Instead of removing chmod entirely (which hurts Mac/Linux users), I replaced it with a small Node.js script that checks process.platform. It now runs chmod +x only if you are NOT on Windows"
Then finally the best parts: The issues have been addressed. except the Claude marketplace.
Apologies for the long commit message