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

Custom rate and named standard rate for units (Issue #1216) #1388

Merged
merged 42 commits into from
Jan 8, 2025

Conversation

gyordong
Copy link
Contributor

@gyordong gyordong commented Nov 17, 2024

Description

(Please include a summary of the change and which issue is touched on. Please also include relevant motivation and context.)

Fixes #1216

Done in collaboration with @huss, @aduques, @AchsahJojo, @emilysarne.

Type of change

  • Note merging this changes the database configuration.
  • This change requires a documentation update

Checklist

  • I have followed the OED pull request ideas
  • I have removed text in ( ) from the issue request
  • You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

gyordong and others added 26 commits July 11, 2024 11:00
…t value to custom input if nonstandard value
- Update unit edit logic for useEffect more
- Change custom sec in rate to require enter
- Disable save if not changed and incomplete custom input
- use setState and not directy set state
- consistently use === & !==
- Added SHL comments for original developers to look at and then remove
- Formatting
- Minor edit

Note create unit does not have new logic and bar may need
to be update to be similar
Note that @gyordong worked with me on these changes.

- Also has comments with ?? on items to do.
- Not carefully tested/completed.
…ts on customratevalid(), included customratevalid in editunit's canSave useEffect check to fix a bug with negative numbers.
@gyordong gyordong changed the title 1216 Custom rate and named standard rate for units (Issue #1216) Nov 17, 2024
Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

I want to thank @gyordong, @aduques, @AchsahJojo & Emily Sarne for this contribution. Is Emily Sarne the GitHub user @emilysarne? I want to verify they have signed the CLA if they contributed to this work.

Testing found this works as desired. Most of my comments relate to differences between the code for edit and create for unit. I welcome your thoughts on my comments and you can address if you think a comment is fine. Please let me know if I can help in any way.

Copy link
Member

@huss huss left a comment

Choose a reason for hiding this comment

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

@gyordong The changes you made looked good and worked in almost every case. Since it was complex to note all my ideas and to move this forward, I pushed a commit that aligned unit edit and create. With this, I think this PR is now ready to merge. I'll let it sit for a little while to give everyone involved a chance to look at what I did and comment if they have nay concerns. If nothing needs changing then it will be merged as is. Congratulations on completing this work and I appreciate your efforts to do address all the comments given changes over time.

@huss huss merged commit 355bbb7 into OpenEnergyDashboard:development Jan 8, 2025
3 checks 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.

standard choices for sec. in rate
4 participants