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 with AppName stripping incorrectly. #379

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blakef
Copy link
Contributor

@blakef blakef commented Feb 26, 2024

Handles cases where appName wasn't playing nicely with it's default value.

Summary

The appName is meant to be stripped off in various places. When setting the default, the actual value passed around is ''. This change does 2 things:

  1. makes regex stricter to only match against prefixes, and
  2. passes the default appName down the tree when an empty string is inputted.

Before

CleanShot 2024-02-26 at 16 05 45@2x

After

CleanShot.2024-02-26.at.22.35.52.mp4

Test Plan

yarn start

What are the steps to reproduce?

You can see it live.

Checklist

  • I tested this thoroughly
  • I added the documentation in README.md (if needed)

@blakef blakef marked this pull request as draft February 26, 2024 16:27
@blakef
Copy link
Contributor Author

blakef commented Feb 26, 2024

Well that's note entirely working 😉... BRB. Fixed.

@blakef blakef force-pushed the fix-appname-stripping branch from cca3815 to 4e71a7f Compare February 26, 2024 22:31
@blakef blakef marked this pull request as ready for review February 26, 2024 22:40
When appName or appPackage are empty, set the state to the default
values. This makes the down-tree props simpler to understand.

Also more strictly strip the prefix AppName from the filepath to avoid
edgecases.
@blakef blakef force-pushed the fix-appname-stripping branch from 4e71a7f to 7ef8fb7 Compare February 26, 2024 22:42
@frankcalise
Copy link
Contributor

Should close #381 and #382

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