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

Fixed typo - "Stay focus" #597

Merged
merged 2 commits into from
Apr 6, 2024
Merged

Fixed typo - "Stay focus" #597

merged 2 commits into from
Apr 6, 2024

Conversation

ZelnickB
Copy link
Contributor

In several locations, the app uses the phrase "Stay focus," which should say "Stay focused" (grammatical error). The commit in this pull request resolves the issue.

For clarity, the names of variables and functions that use the words "stay" and "focus" have also been updated.

@sekwah41
Copy link
Member

Hey, sorry though a large pr was just merged that needed reviewing for a few weeks, could you possibly refactor your changes on top of the new master branch?

@ZelnickB ZelnickB closed this Mar 12, 2024
@ZelnickB
Copy link
Contributor Author

@sekwah41 Done.

@ZelnickB ZelnickB reopened this Mar 12, 2024
@ZelnickB ZelnickB requested a review from sekwah41 March 13, 2024 02:54
Copy link
Member

@sekwah41 sekwah41 left a comment

Choose a reason for hiding this comment

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

Looks good, will test it tonight!

@@ -50,9 +50,9 @@ const CounterProvider: React.FC = ({ children }) => {

const [shouldFullscreen, setShouldFullscreen] = useState(false);

const [count, setCount] = useState(config.stayFocus * 60);
Copy link
Member

Choose a reason for hiding this comment

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

You can change the litteral translations but can you revert the variables. The issue is that these are stored as a json so it means the settings don't work/exist and result in an NAN for existing users
image

Copy link
Member

Choose a reason for hiding this comment

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

image

@sekwah41
Copy link
Member

To change the stay-focused variable itself, we'd need to load it from both the old naming and the new naming to allow it to transition users over slowly. While its an option its a small price to pay for a value users will never see.

Variable names have been reverted to pre-49941a16.
@ZelnickB
Copy link
Contributor Author

@sekwah41 Fixed.

@ZelnickB ZelnickB requested a review from sekwah41 March 19, 2024 02:53
@sekwah41
Copy link
Member

sekwah41 commented Apr 2, 2024

Sorry for the delay, ill take a look at this tomorrow ^.^

@sekwah41 sekwah41 merged commit 9c5cbd6 into zidoro:master Apr 6, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed/Done
Development

Successfully merging this pull request may close these issues.

2 participants