Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

Conversation

a-thansen
Copy link
Contributor

@a-thansen a-thansen commented Nov 5, 2023

Progress Report

06/11

image

Bugs:

  • minimizing windows to less than 712 px window width leads to navbar squashing
  • selecting >2 condition indicators always defaults to the data shown in screenshot

Features to add:

  • Allow for a select all functionality instead of 'ALL'

@a-thansen a-thansen added bug Something isn't working frontend Frontend related Main map Features related to the main map page Inspect map Features related to the inspect map page labels Nov 5, 2023
@a-thansen a-thansen added this to the Release 2 milestone Nov 5, 2023
@a-thansen a-thansen self-assigned this Nov 5, 2023
@a-thansen a-thansen force-pushed the 38-create-a-dropdown-list-allowing-multiple-items-selected-at-the-same-time branch 2 times, most recently from 3746121 to 3c1915d Compare November 6, 2023 11:43
@a-thansen a-thansen marked this pull request as draft November 6, 2023 14:47
@a-thansen a-thansen force-pushed the 38-create-a-dropdown-list-allowing-multiple-items-selected-at-the-same-time branch 2 times, most recently from abb162f to 1715e5d Compare November 6, 2023 20:16
@a-thansen a-thansen removed the bug Something isn't working label Nov 6, 2023
@a-thansen
Copy link
Contributor Author

a-thansen commented Nov 7, 2023

This should be done now and ready for use w plots for example, made a new issue concerning the data bug in the description and etc. #106

@a-thansen a-thansen marked this pull request as ready for review November 7, 2023 07:22
@a-thansen a-thansen force-pushed the 38-create-a-dropdown-list-allowing-multiple-items-selected-at-the-same-time branch from cd38273 to 6cc7ab2 Compare November 7, 2023 07:35
@marcosantiagomuro
Copy link
Contributor

marcosantiagomuro commented Nov 7, 2023

so I suppose that when we select ALL to should be equal to having KPI + IRI + DI + E_norm, right?

KPI + IRI + DI + E-norm:
image

ALL:
image

apparently ALL shows even more indicators? is it correct?

Copy link
Contributor

@marcosantiagomuro marcosantiagomuro left a comment

Choose a reason for hiding this comment

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

looks very smooth, I don't know what ALL should mean, if it includes parameters that we don't show in the dropdown menu than it wouldn't make that much sense in my opinion (so we should add also those). if instead it is only a problem that the existing parameters are not shown then it's fixable somewhere I suppose

@marcosantiagomuro
Copy link
Contributor

I pulled and now with mu seems to be working (?) :)

@marcosantiagomuro
Copy link
Contributor

also maybe there is a layering problem?

If I first select KPI:

image

then I add IRI. which adds it to the layer of KPI:
so if on the same road segments where a KPI was taken we have an IRI value, the IRI just goes on "top" of it.

image

but actually IRI has more road segments than KPI, but I think that for some layering options it stays hidden.
image

also if we have both KPI and IRI do the colours add up ?

because KPI has only red, IRI only yellow,
but together we can also see some green

@a-thansen
Copy link
Contributor Author

looks very smooth, I don't know what ALL should mean, if it includes parameters that we don't show in the dropdown menu than it wouldn't make that much sense in my opinion (so we should add also those). if instead it is only a problem that the existing parameters are not shown then it's fixable somewhere I suppose

I'm thinking in the next version of multi-select, I'll incorporate a select all feature which would allow all the conditions to be auto-selected, or setting up a feature that when you select "ALL" the rest of the options cannot be selected

@a-thansen a-thansen force-pushed the 38-create-a-dropdown-list-allowing-multiple-items-selected-at-the-same-time branch 2 times, most recently from a416f8a to 5764fe1 Compare November 7, 2023 11:08
Copy link
Contributor

@Seb-sti1 Seb-sti1 left a comment

Choose a reason for hiding this comment

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

I add a comment to reduce the complicatedness but that's not a big deal

frontend/src/models/conditions.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@Seb-sti1 Seb-sti1 left a comment

Choose a reason for hiding this comment

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

I add a comment to reduce the complicatedness but that's not a big deal

Sorry I forgot to add that I remembered recently that we shouldn't be creating functions in Functional Component in React. The most likely alternative in our case would be useCallback see this https://stackoverflow.com/questions/46138145/where-should-functions-in-function-components-go

@a-thansen a-thansen force-pushed the 38-create-a-dropdown-list-allowing-multiple-items-selected-at-the-same-time branch from 5764fe1 to 3001ba0 Compare November 8, 2023 13:35
Copy link
Contributor

@Seb-sti1 Seb-sti1 left a comment

Choose a reason for hiding this comment

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

🚀

@a-thansen a-thansen merged commit 9690667 into dev Nov 8, 2023
6 checks passed
@a-thansen a-thansen deleted the 38-create-a-dropdown-list-allowing-multiple-items-selected-at-the-same-time branch November 8, 2023 13:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frontend Frontend related Inspect map Features related to the inspect map page Main map Features related to the main map page
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a dropdown list allowing multiple items selected at the same time
3 participants