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

Bugfixing 10.3 #1659

Merged
merged 4 commits into from
Feb 28, 2021
Merged

Bugfixing 10.3 #1659

merged 4 commits into from
Feb 28, 2021

Conversation

chaveiro
Copy link
Contributor

Fix mysql DMY problem

  • Improvement of zoom graph viz
  • Improvement of Input list on small devices and with big descriptions.

- Fix mysql DMY problem
- Improvement of zoom graph viz
- Improvement of Input list on small devices and with big descriptions.
@chaveiro chaveiro closed this Feb 27, 2021
@chaveiro chaveiro reopened this Feb 27, 2021
@chaveiro
Copy link
Contributor Author

Please merge, more to come later.

@TrystanLea
Copy link
Member

Hello @chaveiro thanks for this, would you mind breaking this down into each individual feature, I find the mix of different things really difficult to evaluate.

I think this would work well:

  • Pull request 1: Mysql engine bug fix
  • Pull request 2: input view changes (include screenshots of what the changes achieve and why they are needed).
  • Pull request 3: vis api.js change (i think its just a change of order?)
  • Pull request 4: multigraph changes (im not sure whats going on in those changes, a bit of a description on that would be great)
  • Pull request 5: zoom interface changes (screenshots and explanation again of what's changed would be really helpful)

I feel like my past practice of merging pull request blind has made the emoncms code base something that I no longer understand and makes it really hard to maintain and continue to develop. Some of it is also my own doing of course but really keen to try to get back to something more maintainable.

@TrystanLea
Copy link
Member

Could you also give me some visibility as to your proposed up and coming changes?

It would be good to check that we are not planning parallel efforts and to find that we get to a point were we are both working on the same part and need to merge two different streams of work together.

One example is the feed list refactor (#1658) which I plan to replicate on the inputs list which would remove your changes above alongside the rest of the js I introduced there to create the responsive view.

- Feedview  small fixes
- Mysql engine refactor and small fix, return null instead of 0 on data missing
Fix touch on some Vis.
@chaveiro
Copy link
Contributor Author

chaveiro commented Feb 28, 2021

Hi, this are just minor fixes to 'in my face' bugs.
No real plan for future features for now.
I see the code got a really mess since the last 4 years... Ex the inputs and feeds views are totally different in implementation with redundant code.
For the mysql engine i was comparing to my 4 years version and just fix a simple issue that were returning a null on the DMY result and also refactor to the template that exists so i can compare the files side by side.
Zoom fix was to add kwh/m /y /d and price where relevant. also fix some NaN issues that were appearing.
Please accept this commit and i will take more careful breaking context on the next ones, all tested.

@TrystanLea TrystanLea merged commit 815a630 into emoncms:master Feb 28, 2021
@TrystanLea
Copy link
Member

Ok I've merged this one, I notice there is a gap opened on the feeds page, just under the controls:

image

do you see the same?

@chaveiro
Copy link
Contributor Author

Its caused by

Why is there, can be removed ?

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