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

Add a filter panel #196

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Add a filter panel #196

wants to merge 36 commits into from

Conversation

mi-sts
Copy link
Collaborator

@mi-sts mi-sts commented May 12, 2023

Close #42, close #43, close #181
filter_panel

@Alexander-Ploskin
Copy link
Collaborator

Problems:

  1. Don't filter images if there are no filters, now user should add dumb filter manually if he doesn't need filters.
    tcaUgjmZLD

  2. Filters is shared between different projects.

  3. Filters is not applied when project was imported.
    qo7vBiPJH4

  4. Filter data dropped when filter was disabled.
    bImTZUWjcv

  5. Show images with landmark doesn't work at all.
    pNsTRIj2rl

  6. Multiple disable buttons on each layer.
    yWgAz8L90g

@Alexander-Ploskin
Copy link
Collaborator

Alexander-Ploskin commented Jun 8, 2023

Problems:

  1. It works great when all filters was removed, but it should also work this way when all filters was disabled, else user has to add dumb filter.
    JSJT8MlBr2

  2. Nice to see that filter data was not dropped when I disable a filter, but this should also work when a user closes an editor.
    qRg5cDwRvz

  3. 0 value for "show every image" should not be allowed the same as negative values.
    Je6zehDbhG

  4. Filters disappear when I update any of them.
    7ZUVShAxUi

import tornadofx.launch

class SolveApp : App(MainView::class) {
class SolveApp : App(MainView::class, ApplicationStylesheet::class) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have global application style class Style.kt, can we merge them?

Comment on lines 27 to 29
const val IconsLayersFilled = "/icons/sidepanel/LayersFilled.png"
const val IconsGrid = "/icons/sidepanel/Grid.png"
const val IconsGridSelected = "/icons/sidepanel/GridSelected.png"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Create an issue to convert all icons to svg

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

return null
}

return constraints.first().message
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why only first?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When we adding a constraint for a message, it becomes a first element

rippleGenerator.clipSupplier = Supplier {
RippleClipTypeFactory(RippleClipType.CIRCLE).setRadius(radius).build(this)
}
style {
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be to global stylesheet?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's better to leave it inside, because we have a radius parameter passed as an argument, which is needed for both the button and the ripple factory when button is pressed. It also ensures that no other set of styles will change the settings of the button created in this function.

attachTo(this@controlButton, op)
}

fun EventTarget.dialogHeaderLabel(text: String, op: Label.() -> Unit = {}) = label(text) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use it also in ChooserDialog

paddingRight = 20.5
}
)
Platform.runLater {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure, that it will work always?

}
)
Platform.runLater {
val cellCheckbox = (cell.getChildList()?.firstOrNull { it is CheckBox } as? CheckBox) ?: return@runLater
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be singleOrNull?

Comment on lines +81 to +83
// Needed because when an unselected element is removed,
// the whole selection is recreated with the addition of the remaining elements,
// which value changes causes changes of selectionProperty.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did not understand at all :(

import solve.filters.settings.model.UIDFilterSetting
import solve.project.model.ProjectFrame

class Filter(val settings: List<FilterSetting<out Any>>) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be make it mutable to avoid problems with list view?

Comment on lines +38 to +39
projectController.model.projectProperty.onChange {
applyFilters()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any case when a lot of updates happen at once?

@mi-sts
Copy link
Collaborator Author

mi-sts commented Jun 18, 2023

Problems:

...
4. Filters disappear when I update any of them.
7ZUVShAxUi

This is a problem with the materialfx library, I tried to avoid this behavior for a long time, but nothing helped.

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.

Decrease the width of the catalogue sidepanel Filter applying Filter editor
2 participants