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

Numberless mode #849

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Numberless mode #849

wants to merge 11 commits into from

Conversation

jkap
Copy link

@jkap jkap commented Oct 1, 2024

  • replaces all numbers greater than 1 with "Several"
  • adds a new experimental setting (default false) for this behavior

A quick audit tells me that shortenNumber is consistently used everywhere I would want to change, so I cut out the middleman and did it there.

Genuinely unsure if this is the best way to load a setting within a non-component context. Will gladly change if needed. Nearly immediately after opening this PR I realized the (likely) correct way. Let me know if I'm still off-base.

jkap added 3 commits October 1, 2024 14:28
- replaces all numbers greater than 1 with
  "Several"
- adds a new experimental setting (default
  `false`) for this behavior

A quick audit tells me that `shortenNumber` is
consistently used everywhere I would want to
change, so I cut out the middleman and did it
there.
@Sorixelle
Copy link

In my testing, I've spotted a couple places where some metrics are still visible with this option enabled:

@qt-dork
Copy link

qt-dork commented Oct 3, 2024

woo thank you jae. excited for this to merge it literally solves all my problems

@ticky
Copy link

ticky commented Oct 3, 2024

Just a note, faving or unfaving a post currently animates in a "1" before hiding it on your deployment.

@Steffo99
Copy link
Contributor

Steffo99 commented Oct 4, 2024

Used this a bit in my fork and seems to be working fine! 🤩

Text doesn't seem to be updated everywhere like it is when Cloak mode is activated though, I wonder what's the reason for that...

cheeaun and others added 8 commits October 9, 2024 11:57
@cheeaun cheeaun added the enhancement New feature or request label Oct 10, 2024
@cheeaun
Copy link
Owner

cheeaun commented Oct 10, 2024

2 notes:

  • Returning Several from shortenNumber is kinda hacky 😬
    • i18n gets a little tricky though not entirely impossible
    • Thinking about this, might as well "cloak" all numbers? (There's Cloak mode in Phanpy which hides text, useful for taking screenshots)
  • For some reason, the recent commits seems co-authored, became part of this PR, and the changes are not specific to this feature.

@jkap
Copy link
Author

jkap commented Oct 10, 2024

  • Returning Several from shortenNumber is kinda hacky 😬

unbelievably hacky! no question there. the use of "several" is a joke relating back to how we handed numbers on cohost.

  • Thinking about this, might as well "cloak" all numbers? (There's Cloak mode in Phanpy which hides text, useful for taking screenshots)

this is almost certainly the better way to go. I can look into switching to this

  • For some reason, the recent commits seems co-authored, became part of this PR, and the changes are not specific to this feature.

git fail on my part; tried to rebase main on and goofed it. will resolve that one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants