Skip to content

Fix UI and add setting options#3

Open
Leto-cmd wants to merge 2 commits into
TheGittyPerson:mainfrom
Leto-cmd:improvements-and-bugfixes
Open

Fix UI and add setting options#3
Leto-cmd wants to merge 2 commits into
TheGittyPerson:mainfrom
Leto-cmd:improvements-and-bugfixes

Conversation

@Leto-cmd
Copy link
Copy Markdown

I've noticed that the text sometimes overlaps with the button, so it is aesthetically unpleasant, so I have changed that. I have also added "feminine" and "neutral" options to the text that comes out, so that a larger audience can use the tool, along with a dark/light mode.

lmk what you think.

Leto

@TheGittyPerson TheGittyPerson self-requested a review March 27, 2026 01:13
Copy link
Copy Markdown
Owner

@TheGittyPerson TheGittyPerson left a comment

Choose a reason for hiding this comment

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

This works very nicely and I don't see any immediate issues. Although could you please move the settings button to the top right corner of the screen to avoid covering the text at the bottom? Thanks!

Copy link
Copy Markdown
Owner

@TheGittyPerson TheGittyPerson left a comment

Choose a reason for hiding this comment

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

Also, I not the most familiar with JS, so here's a brief review of possible issues with main.js, generated with the help of AI. Please let me know if these are valid or worth considering:

Findings

  1. High: button and clickCount are queried at module top level and used immediately (button.addEventListener, button.getBoundingClientRect) without null checks. If the script loads before the DOM is ready or on a page without those elements, this will throw and break the page. Move the queries inside DOMContentLoaded, use defer, or guard for null before using.
  2. High: initSettings assumes #settings-toggle, #settings-modal, #settings-backdrop, and #settings-close always exist. Any missing element will cause a crash when adding listeners or manipulating classes. Guard those elements or short-circuit if any are missing.
  3. Medium: createFloatingMessage only removes the element on transitionend. If the CSS transition is removed or user prefers reduced motion and the transition is disabled, the element may never be removed, leading to DOM growth over time. Add a fallback timeout to remove the element.
  4. Low: On very small viewports, maxOffsetX/maxOffsetY can become negative, which makes the random positioning math awkward. Clamp them to Math.max(0, …) to avoid inverted ranges.

Open Questions / Assumptions

  1. Is this script loaded with defer or at the end of body on all pages that include it? If not, the null access issues will occur.
  2. Are the settings elements guaranteed to exist on every page using this script, or should the logic be optional?

Signed-off-by: Morpheus <167074500+thegittyperson@users.noreply.github.com>
@TheGittyPerson TheGittyPerson changed the title Fixed stuff 👍 Fix UI and add setting options Mar 27, 2026
@TheGittyPerson
Copy link
Copy Markdown
Owner

@Leto-cmd so are you planning on...

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