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

Jon/fix/empty-search #5393

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Jon/fix/empty-search #5393

merged 3 commits into from
Dec 13, 2024

Conversation

Jon-edge
Copy link
Collaborator

@Jon-edge Jon-edge commented Dec 10, 2024

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

Copy link
Contributor

@samholmes samholmes left a comment

Choose a reason for hiding this comment

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

questions/feedback

Comment on lines 63 to 64
/** Manually control the whether the input appears selected. Visual only,
* mutually exclusive with the text input's true blur/focus state. */
Copy link
Contributor

Choose a reason for hiding this comment

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

fixup to the grammar

/** Manually control whether the input appears selected. Visual only; mutually exclusive with the text input's true blur/focus state. */

Copy link
Collaborator Author

@Jon-edge Jon-edge Dec 11, 2024

Choose a reason for hiding this comment

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

No, it's correct. ChatGPT also said this was fine the way it was. If you really wanted a semicolon, it would be in place of the period of the first sentence.

Copy link
Contributor

Choose a reason for hiding this comment

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

ChatGPT is wrong.

src/components/themed/SimpleTextInput.tsx Outdated Show resolved Hide resolved
src/components/themed/SimpleTextInput.tsx Outdated Show resolved Hide resolved
src/components/themed/SimpleTextInput.tsx Outdated Show resolved Hide resolved
src/components/themed/SimpleTextInput.tsx Outdated Show resolved Hide resolved
@Jon-edge Jon-edge force-pushed the jon/fix/empty-search branch 3 times, most recently from d33fcab to 7dd4a5d Compare December 11, 2024 23:53
Copy link
Contributor

@samholmes samholmes left a comment

Choose a reason for hiding this comment

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

Approved, but please fix the comment the whether the ChatGPT says it's okay or not.

@samholmes samholmes force-pushed the jon/fix/empty-search branch from 7dd4a5d to 6c51af2 Compare December 13, 2024 02:05
@samholmes samholmes enabled auto-merge December 13, 2024 02:05
@samholmes samholmes merged commit ee30006 into develop Dec 13, 2024
2 checks passed
@samholmes samholmes deleted the jon/fix/empty-search branch December 13, 2024 02:14
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