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

Brightness device feature Dashboard #1082

Merged
merged 12 commits into from
Mar 22, 2021

Conversation

VonOx
Copy link
Contributor

@VonOx VonOx commented Feb 28, 2021

Pull Request check-list

To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:

  • If your changes affects code, did your write the tests?
  • Are tests passing? (npm test on both front/server)
  • Is the linter passing? (npm run eslint on both front/server)
  • Did you run prettier? (npm run prettier on both front/server)
    - [ ] If you are adding a new features/services, did you run integration comparator? (npm run compare-translations on front)
    - [ ] If your changes modify the API (REST or Node.js), did you modify the API documentation? (Documentation is based on comments in code)
    - [ ] If you are adding a new features/services which needs explanation, did you modify the user documentation? See the GitHub repo and the website.
  • Did you add fake requests data for the demo mode (front/src/config/demo.json) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

This PR add brightness slider to dashboard

image

@codecov
Copy link

codecov bot commented Feb 28, 2021

Codecov Report

Merging #1082 (6319a71) into master (7eeda27) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1082   +/-   ##
=======================================
  Coverage   95.57%   95.57%           
=======================================
  Files         532      532           
  Lines        7166     7168    +2     
=======================================
+ Hits         6849     6851    +2     
  Misses        317      317           
Impacted Files Coverage Δ
...r/services/philips-hue/lib/light/light.setValue.js 90.47% <100.00%> (+1.00%) ⬆️

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 7eeda27...6319a71. Read the comment docs.

@atrovato
Copy link
Contributor

Nice to have, you use a simple slider, be carefull with device long names.

@VonOx VonOx changed the title WIP - Brighntess device feature WIP - Brightness device feature Dashboard Feb 28, 2021
@VonOx VonOx marked this pull request as ready for review February 28, 2021 18:05
@VonOx VonOx requested a review from atrovato February 28, 2021 18:05
@VonOx VonOx changed the title WIP - Brightness device feature Dashboard Brightness device feature Dashboard Feb 28, 2021
Copy link
Contributor

@Pierre-Gilles Pierre-Gilles left a comment

Choose a reason for hiding this comment

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

Very nice PR! thanks 👏

I tried it with the MQTT service, it works fine, except one thing, it's weird the bar is displayed all in blue ?

Screenshot 2021-03-01 at 11 46 57

It's weird because on your screenshot, it's fine.

I'm using Chrome 88

@VonOx
Copy link
Contributor Author

VonOx commented Mar 1, 2021

Oh!

This is the same slider from multi level device. So broken too on Chrome.
Screenshot has been taken from latest Firefox.

@VonOx
Copy link
Contributor Author

VonOx commented Mar 1, 2021

Confirmed on chrome, is this tabler related issue ?

@Pierre-Gilles
Copy link
Contributor

It's probably a tabler issue, it seems to work in the latest version of tabler:

Screenshot 2021-03-01 at 18 00 48

Maybe let's find what's different between the two version and apply a patch here ?

@VonOx
Copy link
Contributor Author

VonOx commented Mar 8, 2021

Not something as simple, in your version input is styled. Latest tabler use nouislider plugin and style are applied on div.

I let you decide what to do, but maybe this chrome behavior is something acceptable ( the slider works ). And when tabler will be updated ( a lot of work ) slider will be ok on chrome.

Copy link
Contributor

@Pierre-Gilles Pierre-Gilles left a comment

Choose a reason for hiding this comment

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

Ok to merge

@Pierre-Gilles Pierre-Gilles merged commit b8db0f2 into GladysAssistant:master Mar 22, 2021
@VonOx VonOx deleted the light-brightness branch April 20, 2021 08:44
Jean-PhilippeD pushed a commit to Jean-PhilippeD/Gladys that referenced this pull request Oct 13, 2021
Co-authored-by: Pierre-Gilles Leymarie <pierregilles.leymarie@gmail.com>
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.

None yet

3 participants