Skip to content

Conversation

@BelKed
Copy link
Contributor

@BelKed BelKed commented Feb 22, 2025

Important

Handle undefined browser name and hostname in UI by setting default values and improve type handling in settings and storage modules.

  • UI Handling:
    • In src/popup/main.ts, set default text to 'unknown' for browserNameElement and hostnameElement if values are undefined.
  • Settings Management:
    • In src/settings/main.ts, add checks for hostnameInput existence before accessing its value.
    • Modify form submission to prevent default and cast event to SubmitEvent.
  • Storage Access:
    • In src/storage.ts, add StorageData type for better type handling in getBrowserName and getHostname functions.

This description was created by Ellipsis for f2bceda. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Reviewed everything up to f2bceda in 1 minute and 7 seconds

More details
  • Looked at 86 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 12 drafted comments based on config settings.
1. src/popup/main.ts:84
  • Draft comment:
    Use of '??' to provide a fallback for undefined browserName improves robustness. Ensure that an empty string isn't a valid value.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. src/popup/main.ts:90
  • Draft comment:
    Consistently providing a fallback for undefined hostname enhances UI reliability.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. src/settings/main.ts:27
  • Draft comment:
    Checking for hostnameInput existence before accessing its value is a good defensive check.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. src/settings/main.ts:107
  • Draft comment:
    Using an inline arrow function to prevent default behavior on form submission ensures cleaner handling.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
5. src/storage.ts:91
  • Draft comment:
    Casting the object returned from browser.storage.local.get using a StorageData type improves type clarity.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. src/storage.ts:97
  • Draft comment:
    Consistent approach applied for getHostname using nullish casting ensures similar behavior.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
7. src/popup/main.ts:84
  • Draft comment:
    Good use of the nullish coalescing operator to default undefined browser names to 'unknown'. Consider extracting 'unknown' to a constant if this default is used elsewhere.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. src/popup/main.ts:90
  • Draft comment:
    Similarly, defaulting hostname to 'unknown' using ?? prevents displaying undefined values. Looks correct.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
9. src/settings/main.ts:107
  • Draft comment:
    The event listener now calls e.preventDefault() before invoking saveOptions, which itself also calls e.preventDefault(). This duplication is redundant—consider removing one to keep the flow clear.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
10. src/settings/main.ts:26
  • Draft comment:
    Early returns when hostname input is missing prevent errors, but failing silently may hinder debugging. Consider logging a warning or error when the element is not found.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
11. src/storage.ts:91
  • Draft comment:
    Enhanced type safety using an explicit StorageData type for getBrowserName improves clarity and robustness.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
12. src/storage.ts:97
  • Draft comment:
    Consistent type casting for getHostname using StorageData ensures that unexpected types are handled properly.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None

Workflow ID: wflow_cWHDGagh1atCWCHh


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@ErikBjare ErikBjare merged commit 0232562 into ActivityWatch:master Feb 22, 2025
5 checks passed
@ErikBjare
Copy link
Member

Nice, thanks!

@BelKed BelKed deleted the typecheck branch March 12, 2025 14:21
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