search

getodk / briefcase

ODK Briefcase is a Java application for fetching and pushing forms and their contents. It helps make billions of data points from ODK portable. Contribute and make the world a better place! ✨💼✨
https://docs.getodk.org/briefcase-intro
Other
60 stars 154 forks source link

Buttonclass #845

Closed jay4kelly closed 4 years ago

jay4kelly commented 4 years ago

Closes #838

What has been done to verify that this works as intended?

Tested using UI for all buttons and chevrons.

Why is this the best possible solution? Were any other approaches considered?

Created classes called DetailsStatusButton and ExportConfigurationButton. The selection column didn't need a class for it as there was no business logic incorporated. So we continued to use a compareSelectionfunction inside the UI class.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Localized just to the UI tests conducted.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

No

codecov-io commented 4 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@fd229a4). Click here to learn what that means. The diff coverage is 7.95%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #845   +/-   ##
=========================================
  Coverage          ?   48.29%           
  Complexity        ?     1637           
=========================================
  Files             ?      195           
  Lines             ?    10360           
  Branches          ?      747           
=========================================
  Hits              ?     5003           
  Misses            ?     5001           
  Partials          ?      356
Impacted Files Coverage Δ Complexity Δ
...briefcase/ui/reused/ExportConfigurationButton.java 0% <0%> (ø) 0 <0> (?)
...ase/ui/reused/transfer/TransferFormsTableView.java 0% <0%> (ø) 0 <0> (?)
...i/reused/transfer/TransferFormsTableViewModel.java 0% <0%> (ø) 0 <0> (?)
...takit/briefcase/ui/reused/DetailsStatusButton.java 0% <0%> (ø) 0 <0> (?)
src/org/opendatakit/briefcase/ui/reused/UI.java 45.16% <0%> (ø) 8 <0> (?)
...ndatakit/briefcase/ui/reused/ButtonProcessing.java 0% <0%> (ø) 0 <0> (?)
...i/export/components/ExportFormsTableViewModel.java 29.87% <28.57%> (ø) 9 <0> (?)
...ase/ui/export/components/ExportFormsTableView.java 85.1% <71.42%> (ø) 4 <0> (?)

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 fd229a4...ff1d5d4. Read the comment docs.

jay4kelly commented 4 years ago

@ggalmazor Changed DetailStatusButton and ExportConfigurationButton to not extend JButton. Added ButtonProcessing Interface. Could have added a CommonButton class for the button classes to extend instead of using the interface. I think this would require us to remove the static create method on the button classes and use a new operator instead.

ggalmazor commented 4 years ago

Hi, @jay4kelly!

After exploring a bit more the changes I was suggesting, I've come to the conclusion that they don't bring that much value overall.

This PR is looking good to me now :) Thanks for your work!!

jay4kelly commented 4 years ago

Closes #838

kkrawczyk123 commented 4 years ago

Tested with success! I haven't' noticed any regression. I have confirmed that the issue identified in #830 has been fixed. Verified on Ubuntu, MacOS and Windows.

@opendatakit-bot unlabel "needs testing" @opendatakit-bot label "behavior verified"

ggalmazor commented 4 years ago

Hi, @jay4kelly! I've rebased your branch on top of master to be able to merge it. Thanks for your work!