Improve filter based on feedback #27

brauliorivas · 2024-06-02T01:15:50Z

BEGINRELEASENOTES

The filter menu is now smaller (numerical inputs take less space).
The simStatus options are now populated based on the data, meaning that options are extracted from the json.
A very important fix for filters, now when dragging particles, if there was a filter applied, the filtered particles remain the same. Before, if someone dragged a particle, then filters effect will disappear.
Some functions were moved to new files, and now objects are used to pass one or another type of particles dynamically.

ENDRELEASENOTES
These improvements were discussed at the meeting on May 31.

…ically (particles and links)

…e dragged

github-actions · 2024-06-02T01:16:10Z

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-06-12 21:47 UTC

brauliorivas · 2024-06-02T21:23:03Z

I've just tested this using the CLD simulation and it works well. The simulator statuses are varied (other than 0).

tmadlener · 2024-06-03T09:24:33Z

Nice, can we also have a filter for generator status?

tmadlener · 2024-06-03T09:45:49Z

For a very nitpicky comment, the fields that take numerical values should have units on them to not be confusing.

kjvbrt · 2024-06-04T12:28:19Z

This way the filter should get even more condensed vertically, but I'm not sure if it will work well with the text boxes in the upper part.
Can you try to do the sim/gen status part first?

kjvbrt · 2024-06-04T12:33:37Z

Can you make the filter trigger on hitting enter?

kjvbrt · 2024-06-04T12:40:20Z

Also, remove the exclamation marks from ¡Welcome to dmX! :)

brauliorivas · 2024-06-05T01:08:42Z

Hey @kjvbrt, I've implemented your suggestions (remove exclamation marks and filter on enter press). Also, I want to show you how would the filter look making it more condensed.
Making statuses smaller

Statuses smaller and one line input for numbers

(Sorry, I took too much time, but one event listener was being removed. Never using innerHTML again 😵‍💫. The good part is that now it works as expected)

tmadlener · 2024-06-05T16:08:57Z

js/menu/filter/parameters.js

+const bitFieldDisplayValues = {
+  23: "BitOverlay",
+  24: "BitStopped",
+  25: "BitLeftDetector",
+  26: "BitDecayedInCalorimeter",
+  27: "BitDecayedInTracker",
+  28: "BitVertexIsNotEndpointOfParent",
+  29: "BitBackscatter",
+  30: "BitCreatedInSimulation",
+};


Very nice. The one "complaint" I have about this is that now the BitfieldCheckbox is now very tightly tied to these bitfield values and names. However, these are only correct for the MCParticle.simulationStatus. Hence, I think it would be nice to make the BitfieldCheckbox slightly more generic, such that this map can be passed on construction. Then when we construct the specific filter box for the simulationStatus in the MCParticle page, we can pass in this list and get the dedicated filter box for that. For other bit field checkboxes we would then just need to pass another map and would not have to re-implement this again.

Oh, right! I'm changing this now.

tmadlener · 2024-06-05T16:11:32Z

js/menu/filter/parameters.js

+  23: "BitOverlay",
+  24: "BitStopped",
+  25: "BitLeftDetector",
+  26: "BitDecayedInCalorimeter",
+  27: "BitDecayedInTracker",
+  28: "BitVertexIsNotEndpointOfParent",
+  29: "BitBackscatter",
+  30: "BitCreatedInSimulation",


Suggested change

23: "BitOverlay",

24: "BitStopped",

25: "BitLeftDetector",

26: "BitDecayedInCalorimeter",

27: "BitDecayedInTracker",

28: "BitVertexIsNotEndpointOfParent",

29: "BitBackscatter",

30: "BitCreatedInSimulation",

23: "Overlay",

24: "Stopped",

25: "LeftDetector",

26: "DecayedInCalorimeter",

27: "DecayedInTracker",

28: "VertexIsNotEndpointOfParent",

29: "Backscatter",

30: "CreatedInSimulation",

…n gen and sim

tmadlener · 2024-06-06T07:24:36Z

js/menu/filter/builders.js

+  }
+
+  setCheckBoxes() {
+    this.checkBoxes = Array.from(this.uniqueValues).map(


I think this should map over the dictionary. Even if that would mean that all the boxes are always there regardless of the values that are found in the file.

If you want to do this dynamically, you would have to put in a bit more logic here, because the uniqueValues will be integers, but you would actually have to check in those uniqueValues which are the unique bits that are set (in all of them) and then only display the bit field values that you found. That is also a possibility, but for code simplicity and maintanability, I would (at least for now) do the the easy thing and just take the dictionary.

tmadlener · 2024-06-06T07:25:40Z

js/menu/filter/filter.js

+let parametersRange = [
+  {
+    property: "momentum",
+    unit: "GeV",
+  },
+  {
+    property: "mass",
+    unit: "GeV",
+  },
+  {
+    property: "charge",
+    unit: "e",
+  },
+  {
+    property: "vertex",
+    unit: "mm",
+  },
+  {
+    property: "time",
+    unit: "ns",
+  },
+];


I think this should be the same for all types in EDM4hep, but we might have to check again. Maybe put a comment on top that states that this might need checking again, so that we don't forget?

tmadlener · 2024-06-06T07:26:18Z

js/menu/filter/filter.js

-bitsCheckbox.forEach((checkbox) => checkbox.render(filters));
-
-apply.addEventListener("click", () => {
+const bitFieldDisplayValues = {


Suggested change

const bitFieldDisplayValues = {

const SimStatusBitFieldDisplayValues = {

Just so that it is immediately clear which bit field display values we are talking about here.

kjvbrt · 2024-06-06T08:29:46Z

js/menu/filter/filter.js

+
+const bits = new BitFieldBuilder(
+  "simStatus",
+  "Simulator status",


Simulator -> Simulation

brauliorivas · 2024-06-06T21:14:03Z

Hey @kjvbrt, now that #35 was merged, this last commit deployed just as expected.

tmadlener · 2024-06-07T07:58:03Z

I like the current version a lot. I have just a few nitpicky comments on the appearance.

Can you make Simulation status and Generator status slightly bigger (or bold) in order to stand it out a bit more as headers?
For the Simulation status, I would prefer the check boxes to be in front of the text rather than at the back of it.

tmadlener

This looks good. @kjvbrt anything from your side still?

brauliorivas · 2024-06-12T21:46:19Z

Hi @tmadlener, I'm merging this, because #36 needs to be updated with these changes (#27). I hope @kjvbrt checks when he wants and gives some feedback.

kjvbrt · 2024-06-12T22:29:31Z

Nice work @brauliorivas

brauliorivas added 6 commits May 31, 2024 16:51

remove unnecessary import

8c4950e

make filters more compact

f381347

populate simulation status options based on file

7085a38

apply sort of dependecy inversion, so elements are taken passed dynam…

4f6fd31

…ically (particles and links)

apply same technique for filters

9c643f2

[FIX] when filtering particles, these are maintained even if boxes ar…

e67e6eb

…e dragged

brauliorivas temporarily deployed to github-pages June 2, 2024 01:15 — with GitHub Actions Inactive

[FIX] update test based on new filtering logic (new id's for links)

489bf69

brauliorivas temporarily deployed to github-pages June 2, 2024 01:29 — with GitHub Actions Inactive

brauliorivas added 2 commits June 3, 2024 15:14

create filter for generator status

67bfb6d

add units to range parameters

5c6833a

brauliorivas temporarily deployed to github-pages June 3, 2024 20:24 — with GitHub Actions Inactive

update filters for ranges to require an object

1eed533

brauliorivas temporarily deployed to github-pages June 3, 2024 20:30 — with GitHub Actions Inactive

tmadlener mentioned this pull request Jun 4, 2024

Filtering on simulation status is not yet working as intended. #30

Closed

brauliorivas added 4 commits June 4, 2024 15:04

trigger filter when enter key is pressed

83580b7

remove exclamation mark

d3be20b

display gen status and sim status in a smaller space

1ee4ca8

display each range input in one line

0b27623

brauliorivas temporarily deployed to github-pages June 4, 2024 21:45 — with GitHub Actions Inactive

fix small bug (change from innerHTML to appendChild

49f2bf8

brauliorivas temporarily deployed to github-pages June 5, 2024 01:03 — with GitHub Actions Inactive

brauliorivas temporarily deployed to github-pages June 5, 2024 15:00 — with GitHub Actions Inactive

check simStatus as in EDM4hep + display names instead of number

2769764

brauliorivas temporarily deployed to github-pages June 5, 2024 16:03 — with GitHub Actions Inactive

tmadlener reviewed Jun 5, 2024

View reviewed changes

This was linked to issues Jun 5, 2024

Filtering on simulation status is not yet working as intended. #30

Closed

Add possibility to filter MCParticles via generatorStatus #31

Closed

tmadlener reviewed Jun 5, 2024

View reviewed changes

pass dynamically data for bitfield elements and exchange class betwee…

66222e3

…n gen and sim

brauliorivas temporarily deployed to github-pages June 5, 2024 16:36 — with GitHub Actions Inactive

removed unused imports

73624a3

brauliorivas temporarily deployed to github-pages June 5, 2024 16:39 — with GitHub Actions Inactive

tmadlener reviewed Jun 6, 2024

View reviewed changes

kjvbrt reviewed Jun 6, 2024

View reviewed changes

change names + display dictionary

82904c8

brauliorivas temporarily deployed to github-pages June 6, 2024 21:12 — with GitHub Actions Inactive

bolder title + rearrange checkboxes

ec594c5

brauliorivas temporarily deployed to github-pages June 7, 2024 14:23 — with GitHub Actions Inactive

rearrange only sim status

75428b1

brauliorivas temporarily deployed to github-pages June 7, 2024 14:26 — with GitHub Actions Inactive

tmadlener approved these changes Jun 10, 2024

View reviewed changes

brauliorivas merged commit 797dd08 into key4hep:main Jun 12, 2024
2 checks passed

brauliorivas temporarily deployed to github-pages June 12, 2024 21:46 — with GitHub Actions Inactive

brauliorivas temporarily deployed to github-pages June 12, 2024 21:47 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve filter based on feedback #27

Improve filter based on feedback #27

brauliorivas commented Jun 2, 2024 •

edited by kjvbrt

Loading

github-actions bot commented Jun 2, 2024 •

edited

Loading

brauliorivas commented Jun 2, 2024

tmadlener commented Jun 3, 2024

tmadlener commented Jun 3, 2024

kjvbrt commented Jun 4, 2024 •

edited

Loading

kjvbrt commented Jun 4, 2024

kjvbrt commented Jun 4, 2024

brauliorivas commented Jun 5, 2024

tmadlener Jun 5, 2024

brauliorivas Jun 5, 2024

tmadlener Jun 5, 2024

tmadlener Jun 6, 2024

tmadlener Jun 6, 2024

tmadlener Jun 6, 2024

kjvbrt Jun 6, 2024

brauliorivas commented Jun 6, 2024

tmadlener commented Jun 7, 2024

tmadlener left a comment

brauliorivas commented Jun 12, 2024

kjvbrt commented Jun 12, 2024

	const bitFieldDisplayValues = {
	const SimStatusBitFieldDisplayValues = {

Improve filter based on feedback #27

Improve filter based on feedback #27

Conversation

brauliorivas commented Jun 2, 2024 • edited by kjvbrt Loading

github-actions bot commented Jun 2, 2024 • edited Loading

brauliorivas commented Jun 2, 2024

tmadlener commented Jun 3, 2024

tmadlener commented Jun 3, 2024

kjvbrt commented Jun 4, 2024 • edited Loading

kjvbrt commented Jun 4, 2024

kjvbrt commented Jun 4, 2024

brauliorivas commented Jun 5, 2024

tmadlener Jun 5, 2024

Choose a reason for hiding this comment

brauliorivas Jun 5, 2024

Choose a reason for hiding this comment

tmadlener Jun 5, 2024

Choose a reason for hiding this comment

tmadlener Jun 6, 2024

Choose a reason for hiding this comment

tmadlener Jun 6, 2024

Choose a reason for hiding this comment

tmadlener Jun 6, 2024

Choose a reason for hiding this comment

kjvbrt Jun 6, 2024

Choose a reason for hiding this comment

brauliorivas commented Jun 6, 2024

tmadlener commented Jun 7, 2024

tmadlener left a comment

Choose a reason for hiding this comment

brauliorivas commented Jun 12, 2024

kjvbrt commented Jun 12, 2024

brauliorivas commented Jun 2, 2024 •

edited by kjvbrt

Loading

github-actions bot commented Jun 2, 2024 •

edited

Loading

kjvbrt commented Jun 4, 2024 •

edited

Loading