Skip to content

Implement negative temperature conversion (#44) #49

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.

Sign up for GitHub

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

Jump to bottom
Merged
naveensingh merged 18 commits into from
Jun 19, 2025
Merged

Implement negative temperature conversion (#44) #49

naveensingh merged 18 commits into from
Jun 19, 2025

Conversation

@Lionheart-Y2J
Copy link
Contributor

@Lionheart-Y2J Lionheart-Y2J commented Mar 31, 2025 •
edited by fossifybot-legacy
Loading

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Added +/- button for temperature converter
  • Implemented toggleNegative() method in ConverterView to handle sign changes
  • Set conditional button visibility (only for temperature)

Before/After Screenshots/Screen Record

  • Before: No button for negative values
before_temp
  • After: +/- button appears below numpad (only in temperature converter)
after_temp

Fixes the following issue(s)

Relies on the following changes

  • Updated view_unit_converter.xml layout (new LinearLayout container with id plus_minus_layout)
  • Updated UnitConverterActivity.kt button click handler and visibility logic for the +/-
  • Updated ConverterView.kt with sign toggle method and numpadClicked

Acknowledgement

@Aga-C
Copy link
Member

Aga-C commented Mar 31, 2025

I've tested and found two issues:

  • +/- button should also work when there's no value. Most people first enter -, then the rest of the number.
  • Deleting a negative number causes a crash.

There are also two issues that I'm not sure whether we should address them in this PR:

  • There's no such thing as negative Kelvins and negative Rankines. It shouldn't be possible to enter a negative value for these units.
  • By analogy, you shouldn't allow entering values for Celsius and Fahrenheit that are below absolute zero.

@Lionheart-Y2J
Improve temperature conversion handling
f0a1f03 
- Fixed crash when deleting negative values
- Added support for entering negative sign first
- Implemented temperature limits validation during input:
  * Block negative values for Kelvin and Rankine (minimum 0)
  * Enforce absolute zero limits:
    - Celsius: minimum -273.15
    - Fahrenheit: minimum -459.67
- Added validation during digit entry to avoid going below absolute 0 and also after sign toggle
@Lionheart-Y2J
Copy link
Contributor Author

Lionheart-Y2J commented Mar 31, 2025 •
edited
Loading

I fixed the crash issue (deleting negative value resets to 0) and added the possibility to use the - before entering a number.

Concerning the other remarks, I made sure to enforce temperature limits during input by automatically adjusting values that exceed absolute zero for each unit type: Celsius cannot go below -273.15, Fahrenheit below -459.67, and Kelvin/Rankine are blocked entirely from being negative (minimum 0)

@Lionheart-Y2J
Copy link
Contributor Author

Lionheart-Y2J commented May 23, 2025

Hello @Aga-C
Just checking in to see if there's anything else needed on this PR. I haven't planned to make any changes unless needed so if everything looks good, a review would be much appreciated. Thanks

@naveensingh
Copy link
Member

naveensingh commented May 24, 2025

Hi! @Lionheart-Y2J

I'll do a final review and merge this soon.

Thanks.

@naveensingh naveensingh self-assigned this May 26, 2025
naveensingh
naveensingh requested changes May 31, 2025
View reviewed changes
@naveensingh
Copy link
Member

naveensingh commented May 31, 2025 •
edited
Loading

The functionality itself seems to work. Left some comments and nitpicks regarding the code.

@naveensingh naveensingh removed their assignment Jun 10, 2025
@naveensingh naveensingh self-assigned this Jun 19, 2025
naveensingh
naveensingh approved these changes Jun 19, 2025
View reviewed changes
Copy link
Member

@naveensingh naveensingh left a comment

Choose a reason for hiding this comment

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

Made those changes myself to not delay this anymore.

Thank you!

@naveensingh naveensingh merged commit b6f518c into FossifyOrg:main Jun 19, 2025
3 checks passed
naveensingh added a commit that referenced this pull request Jun 19, 2025
@naveensingh
fix: add btnPlusMinus to getButtonIds()
7cce776 
Forgot to add it after commiting #49 (comment)
@naveensingh naveensingh mentioned this pull request Jun 19, 2025
Merged
naveensingh added a commit that referenced this pull request Jun 19, 2025
@naveensingh
fix: add btnPlusMinus to getButtonIds() (#91)
3fe17c7 
* fix: add btnPlusMinus to `getButtonIds()`

Forgot to add it after commiting #49 (comment)

* style: format code
@naveensingh naveensingh mentioned this pull request Jun 30, 2025
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@naveensingh naveensingh naveensingh approved these changes

Assignees

@naveensingh naveensingh

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add +/- to Temperature Unit Converter

3 participants

@Lionheart-Y2J @Aga-C @naveensingh