Skip to content
This repository has been archived by the owner on Aug 9, 2021. It is now read-only.

Allow filter by recording type in search recording screen #62

Merged
merged 3 commits into from
Sep 25, 2018

Conversation

arthurmcgregor
Copy link
Member

Closes #18

Should be merged after #61 because I didn't manage my branches too well in my fork sorry! It's really only the last commit that is different here.

I've squeezed in the recordingType select box as a b-form-select 
component. I also turned the tagType into b-form-select, rather than 
multiselect which took up too much space (and wasn't needed as only ever 
selecting one option)
@arthurmcgregor arthurmcgregor changed the title Issue18 Allow filter by recording type in search recording screen Sep 25, 2018
Copy link
Member

@mjs mjs 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. Three things:

  1. The query criteria form layout is a little off now. The Device field no longer aligns with the Duration field underneath. Looking at the Search button, it looks like Duration is actually the one that's wrong.
  2. Could your previously selected Recording Type preference be remembered? Grant was keen on this.
  3. If "Audio" or "Video and Audio" is selected can Tag Types and Animals be disabled (and their values ignored). Those don't currently make sense for audio recordings.

I'm happy for 2 and 3 to be done as separate PRs if you prefer.

@arthurmcgregor
Copy link
Member Author

arthurmcgregor commented Sep 25, 2018

The duration field was always slightly out - even before I added the recording type input! But fixed now.

Perhaps merge this PR as is and add 2 and 3 to the backlog...

@mjs
Copy link
Member

mjs commented Sep 25, 2018

Thanks for fixing. Would you mind adding issues for 2 and 3?

@arthurmcgregor
Copy link
Member Author

Issues for 2 and 3 created and added to v1 milestone

@mjs
Copy link
Member

mjs commented Sep 26, 2018

Thanks @arthurmcgregor

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

Successfully merging this pull request may close these issues.

2 participants