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

New results model #1787

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft

New results model #1787

wants to merge 3 commits into from

Conversation

taooceros
Copy link
Member

Summary of the change

  1. Use Dynamic Data to replace our buggy complex ResultCollection
  2. Stop Triggering ResultListBox Visibility when results changes from empty to non-empty or vise versa

Effect:
Surprisingly the second change resolves #586, but will requires further test.
Though, it also introduce some style inconsistent. @onesounds will notice it easily.

image

More weirdly, if we switch to context menu and switch back, the style will become normal as current release.

1. Use DynamicData SourceCache instead of our ResultCollection to reduce code complexity
2. Stop trigger Visibility when changes from empty to not empty or vise versa
@taooceros taooceros requested review from jjw24 and onesounds and removed request for jjw24 January 12, 2023 21:16
@github-actions

This comment has been minimized.

@jjw24
Copy link
Member

jjw24 commented Jan 23, 2023

@taooceros please list your tests

@onesounds please check if ok:

Though, it also introduce some style inconsistent. @onesounds will notice it easily.

@github-actions
Copy link

github-actions bot commented Feb 3, 2023

@check-spelling-bot Report

🔴 Please review

See the 📂 files view or the 📜action log for details.

Unrecognized words (6)

LINQ
mainwindow
Mvvm
onesounds
progressbar
viewupdate

To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:Flow-Launcher/Flow.Launcher.git repository
on the new_results_model branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/4083398394/attempts/1'

To have the bot do this for you, reply quoting the following line:
@check-spelling-bot apply updates.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

@onesounds
Copy link
Contributor

onesounds commented Feb 3, 2023

OK. That margin from 'MiddleSeparator' logic. that logic from 'BaseSeparatorStyle' in base.xaml
Since the separator in the middle is used in common with the History and Context menu, it is separated from the Result.

image

A simple solution is to change the code as follows.

image

However, in this case, the Separator is not displayed in the History view. (Ctrl+H)
I think we can do more research, do you have any good ideas? I have some problems because I don't understand the History part well. (Previously, there was a margin problem in the history menu.)

P.S. please mention the issue that I really need in Discord separately. There(github) are so many notifications that I can't check them all. 😓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants