Skip to content

Conversation

@BryonLewis
Copy link
Collaborator

  • models.py updated the attributes for optional editor object which can include future visualization options for attributes
  • Added in the color option for the attribute timeline/filtering tools
  • Added selection between input box/slider for numeric attributes
AttributesSlider.mp4

@BryonLewis BryonLewis requested a review from marySalvi August 12, 2022 16:43
/>
<div v-if="datatype=== 'number'">
<v-radio-group
:value="editor.type"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this line is causing an error for existing attributes. When you click the edit attribute cog the dialog fails to load and console displays this:
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup, editor probably needs to be checked to see if it exists, editor can be undefined.

Copy link
Collaborator

@marySalvi marySalvi left a comment

Choose a reason for hiding this comment

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

When changing a combobox with an existing value to a slider, the value is lost. Is that behavior we want or would we want the slider positioned at that previous value?

Screenshot from 2022-08-16 13-49-30

outlined
:step="editor.range[1]> 1 ? 1 : 0.01"
type="number"
label="Upper"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change label to 'Max'

outlined
:step="editor.range[0]> 1 ? 1 : 0.01"
type="number"
label="Lower"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change label to 'Min'

outlined
:step="editor.steps> 1 ? 1 : 0.01"
type="number"
label="Slider Resolution"
Copy link
Collaborator

Choose a reason for hiding this comment

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

In would change label to something like 'Slider Step Interval' and the hint to something like 'Slider will increase/decrease by this value'

Comment on lines +165 to +166
:step="typeSettings.steps ? typeSettings.steps
: (typeSettings.range[1] - typeSettings.range[0])/2.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's possible for both typeSettings.steps and typeSettings.range to be undefined and the slider breaks. Probably best to make one or the other mandatory when creating the slider.
Screenshot from 2022-08-16 13-30-58

Screenshot from 2022-08-16 13-30-37

Comment on lines +167 to +168
:min="typeSettings.range[0]"
:max="typeSettings.range[1]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We either need to make the range required or have a default value

@BryonLewis
Copy link
Collaborator Author

When changing a combobox with an existing value to a slider, the value is lost. Is that behavior we want or would we want the slider positioned at that previous value?

Screenshot from 2022-08-16 13-49-30

I think that's okay because I can't guarantee that the new range contains the old value. We auto set a range when they swap so that is why the value changes. Ideally this should be set up in the configuration file once and then values are undefined/0 until the user sets them on a specific track/detection attribute.

Copy link
Collaborator

@marySalvi marySalvi left a comment

Choose a reason for hiding this comment

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

Looks Great!

@BryonLewis BryonLewis merged commit 8397b30 into main Aug 23, 2022
@BryonLewis BryonLewis deleted the numeric-attribute-slider branch August 23, 2022 19:24
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.

2 participants