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 multiple zone selection #228

Merged
merged 3 commits into from
Dec 21, 2021
Merged

Add multiple zone selection #228

merged 3 commits into from
Dec 21, 2021

Conversation

klay2000
Copy link
Contributor

PR to make zone selector a multiselector.

@codecov-commenter
Copy link

codecov-commenter commented Dec 10, 2021

Codecov Report

❗ No coverage uploaded for pull request base (develop@ca1dd6e). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #228   +/-   ##
==========================================
  Coverage           ?   56.22%           
==========================================
  Files              ?       11           
  Lines              ?     2659           
  Branches           ?        0           
==========================================
  Hits               ?     1495           
  Misses             ?     1164           
  Partials           ?        0           
Flag Coverage Δ
unittests 56.22% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca1dd6e...af8279c. Read the comment docs.

@linknum23 linknum23 mentioned this pull request Dec 13, 2021
35 tasks
@Lohrer
Copy link
Collaborator

Lohrer commented Dec 17, 2021

Just checked this out. It's a bit awkward since the action that actually performs the zone update is "close the zone dropdown" AKA click anywhere that's not the zone selection dropdown. Some kind of "Add" or "OK" button would be nice, but it does technically work as-is.

It also doesn't seem to work on the Android app at all, but I like that there's an OK button there so it makes more intuitive sense.

@linknum23 linknum23 self-requested a review December 21, 2021 21:04
@linknum23
Copy link
Contributor

linknum23 commented Dec 21, 2021

Just checked this out. It's a bit awkward since the action that actually performs the zone update is "close the zone dropdown" AKA click anywhere that's not the zone selection dropdown. Some kind of "Add" or "OK" button would be nice, but it does technically work as-is.

We can think about adding a button in the future, both Klayton and I were having trouble getting a button to look good CSS-wise.

It also doesn't seem to work on the Android app at all, but I like that there's an OK button there so it makes more intuitive sense.

Just tested this in the app and it works as expected.

@Lohrer
Copy link
Collaborator

Lohrer commented Dec 21, 2021

We can think about adding a button in the future, both Klayton and I were having trouble getting a button to look good CSS-wise.
Just tested this in the app and it works as expected.

Yeah after testing again I don't get the button that I saw before and things work as expected. I must have tested too soon or maybe grabbed an old commit somehow, I'm not sure.

@klay2000 klay2000 merged commit 38a69f4 into develop Dec 21, 2021
@klay2000 klay2000 deleted the add_multiple_zone_selection branch December 21, 2021 21:52
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.

5 participants