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

815 main window resizing #816

Closed dcbriccetti closed 4 years ago

dcbriccetti commented 4 years ago

Closes #815

Please see the commit comments for details of the changes.

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

Not enough. It looks great on a Mac, but I’ll need help checking on other platforms. Could someone please check that the underline is gone and that the hyperlink still works when Aggregate or Central is the source or target?

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

Using a ContentPane and BorderLayout seems best.

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?

Should have no effect.

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

Merging #816 into master will increase coverage by 0.04%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #816      +/-   ##
============================================
+ Coverage     48.63%   48.68%   +0.04%     
- Complexity     1643     1644       +1     
============================================
  Files           192      192              
  Lines         10319    10313       -6     
  Branches        743      743              
============================================
+ Hits           5019     5021       +2     
+ Misses         4942     4934       -8     
  Partials        358      358
Impacted Files Coverage Δ Complexity Δ
...i/reused/transfer/sourcetarget/target/Central.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...reused/transfer/sourcetarget/source/Aggregate.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...i/reused/transfer/sourcetarget/source/Central.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...reused/transfer/sourcetarget/target/Aggregate.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
.../opendatakit/briefcase/ui/MainBriefcaseWindow.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
src/org/opendatakit/briefcase/ui/reused/UI.java 40.27% <0%> (ø) 9 <0> (ø) :arrow_down:
...g/opendatakit/briefcase/ui/export/ExportPanel.java 50% <0%> (+1.92%) 15% <0%> (+1%) :arrow_up:

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 f7cc413...f3535d3. Read the comment docs.

ggalmazor commented 4 years ago

Perhaps @ggalmazor can fix the unchecked call to put in makeClickable.

I don't see a way to remove that unchecked call. I think we need to @Supress("unchecked") to the overloaded method.

Why not assuming that boolean underline will always be true and simplify the if block? We can extract a method to get the underlined font and isolate the unchecked operation there.

dcbriccetti commented 4 years ago

@ggalmazor Thanks for that. What do you think about removing underlines from links throughout Briefcase?

ggalmazor commented 4 years ago

What do you think about removing underlines from links throughout Briefcase?

I don't have a strong opinion about it. I'll let @yanokwa and @kkrawczyk123 fight for it ;)

kkrawczyk123 commented 4 years ago

I don't have a strong opinion about that too and I am not protesting :)

yanokwa commented 4 years ago

Remove the underline, keep the blue.

dcbriccetti commented 4 years ago

Yo, yo, yo, this is ready to go. (I’m in no rush, but the Github prodder has appeared.)