Skip to content

Conversation

@eranl
Copy link
Contributor

@eranl eranl commented Aug 25, 2025

Does this change need to be optional?

Fixes #1902, fixes #1976.

@Helium314
Copy link
Owner

Yes, this should definitely be optional as I expect it may break flow many people are used to.

@eranl
Copy link
Contributor Author

eranl commented Oct 29, 2025

Added setting.

Also:

  • moved ClipboardHistoryView.clipboardHistoryManager initialization up before initialize() call, and made it lateinit, as it is now (almost) always set before use, and it's essential for functionality. But I can revert if preferred.
  • moved initial sorting to initialize(), after settings are loaded.
  • when this setting is changed, settings will be reloaded twice, in order to make sure they are reloaded before sorting. The alternative is a delay, as in
    GlobalScope.launch {
    delay(10) // need to wait until SettingsValues are reloaded
    .

@Helium314
Copy link
Owner

  • moved ClipboardHistoryView.clipboardHistoryManager initialization up before initialize() call, and made it lateinit, as it is now (almost) always set before use, and it's essential for functionality. But I can revert if preferred.

Definitely fine and makes sense. Not sure why it was actually nullable, while other variable were lateinit from the start (1cbbf49).

This is only triggered when changing the setting in the settings screen, so being inefficient and reloading twice is not an issue.
Tough I would like to avoid having logic in there that's about settings that only change in a settings screen. You could just call KeyboardSwitcher.reloadKeyboard on switch change I think. If you want to keep it, at least add a comment why it's there.

override fun compareTo(other: ClipboardHistoryEntry): Int {
val result = other.isPinned.compareTo(isPinned)
var result = other.isPinned.compareTo(isPinned)
if (Settings.getValues()?.mClipboardHistoryUnpinnedFirst == true) result = -result
Copy link
Owner

Choose a reason for hiding this comment

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

Did you have some issues where SettingsValues were not yet loaded when the list is being sorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, before I removed this line. But then I decided to keep this safe call just in case.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks! Reproduced and fixed by loading settings before calling ClipboardHistoryManager.onCreate

@eranl
Copy link
Contributor Author

eranl commented Nov 2, 2025

You could just call KeyboardSwitcher.reloadKeyboard on switch change I think.

That doesn't work, because the onSharedPreferenceChanged call order is still random. It also causes a switch to the abc keyboard if you're on clipboard history.

add a comment why it's there.

I have.

Reorder comparison logic.
@eranl
Copy link
Contributor Author

eranl commented Nov 2, 2025

Reordered comparison logic, to make it slightly more efficient, and maybe a bit clearer.

@Helium314
Copy link
Owner

It also causes a switch to the abc keyboard if you're on clipboard history.

I think that doesn't matter, but the onSharedPreferenceChanged definitely does.

@Helium314 Helium314 merged commit b44e006 into Helium314:main Nov 4, 2025
1 check passed
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.

Clipboard sorting order Option to show recent, unpinned clipboard queries on top, above the pinned ones

2 participants