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

Analog Sensor API Easyintegrate #139

Merged
merged 14 commits into from
Aug 5, 2023

Conversation

opensprinklershop
Copy link
Contributor

Hi Samer,
now you can easily merge the api files.
But again, you can't use uglify for that, use terser

@salbahra
Copy link
Member

Thank you for the revised PR! I started reviewing it and ran into a lot of lint issues.

  1. Could you please resolve the issues highlighted by npx grunt? I notice for example you reference variables defined in other files. In this case, you can simply add the variables to the top comment.

  2. index.html was not updated to reference the additional files and therefore an error is thrown as soon as the page loads (due to undefined variables).

  3. There appears to be a missing image asset (maybe more). It's looking for check-black.png.

  4. Minor UI issues but it would be nice to use the table's in View Logs in the analog sensor config page for consistency. The analog sensor config page does not render well on mobile and most of the table is cut off. We want to ensure everything works from the mobile device.

The graphs on the sensor logs page look great and maybe something we can incorporate for other aspects of the app.

Thanks again and look forward to seeing the things mentioned updated!

@salbahra salbahra self-requested a review August 5, 2023 22:13
@salbahra salbahra merged commit 211ec2b into OpenSprinkler:master Aug 5, 2023
1 check passed
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

2 participants