Deprecate external link and reactable to reuse from shinyGovstyle

[cjrace]

Brief overview of changes

The logic from dfeshiny::external_link() and dfeshiny::dfe_reactable() has been moved to shinyGovstyle.

This PR deprecates the old functions from dfeshiny, warning users of their impending removal, as well as switching the logic to just wrap around the shinyGovstyle functions so that there's only one place to maintain that code in the short term while we're deprecating.

Also tidied up a few linting and other warnings as I went around the package.

Why are these changes being made?

The functions are more widely useable across departments, so was worth moving to shinyGovstyle. In response we want to avoid duplicated effort so deprecating here is the best way to gently move users across while instantly removing the duplicated code.

Detailed description of changes

• Bumped R dependency to >4.1.0 given we use the base R pipe |>
• Fixed spelling errors flagged by spelling package checks
• Remove explicit return() calls where we're expected to use the implicit version instead
• Add an undeclared import for install.packages()
• Moved external_link() to wrap around shinyGovstyle::external_link() and marked as deprecated
• Moved dfe_reactable() to wrap around shinyGovstyle::govReactable(), marked as deprecated, and updated tests to be less specific on HTML output, in line with best practice moving to a snapshot approach for the more detailed checks
• Bumps to v0.6.0
• Updates the snapshots and expectations in the test dashboard UI tests, I'm happy all the changes are as expected

Additional information for reviewers

Expecting @rmbielby will review as dfeshiny owner, though @mzayeddfe may be interested to see this as an example too as we've not done this moving of functions between packages in this way before.

Versioning

This deprecation doesn't break (or really change) anything; it's an official, explicit, advance warning that something will change in a future version, and behind the scenes we've cleaned out the duplicated logic so will get any updates from shinyGovstyle directly.

The SemVer specification specifically says to bump the minor version when deprecating in a backwards compatible way. It's only necessary for >v1.0.0, as v0.x.x is treated as free development reign, but given the adoption already of the package, and the chance for us to practice an example of this, I've gone for the same approach we'd use if the package was fully established and past v1.0.0 already.

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. It MUST be incremented if any public API functionality is marked as deprecated. It MAY be incremented if substantial new functionality or improvements are introduced within the private code. It MAY include patch level changes. Patch version MUST be reset to 0 when minor version is incremented.

I've suggested we intentionally break both functions with lifecycle::deprecate_stop() at v1.0.0, which will just error for users and effectively remove the functions while leaving a clear message. This fits with their suggestion for when to bump the major version number.

Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API. It MAY include minor and patch level changes. Patch and minor version MUST be reset to 0 when major version is incremented.

Issue ticket number/s and link

Resolves #85

[codecov]

Codecov Report

❌ Patch coverage is 87.17949% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.70%. Comparing base (091fe2e) to head (de33300).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
- Coverage   65.54%   63.70%   -1.84%     
==========================================
  Files          15       15              
  Lines        1364     1295      -69     
==========================================
- Hits          894      825      -69     
  Misses        470      470              

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 091fe2e...de33300. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rmbielby]

One question in the comments and then main thought is around the deprecation warnings for dfeshiny functions using these deprecated functions. I've not got much familiarity with deprecation practices, but feel like the other dfeshiny functions flagging warnings for external_links seems sub-optimal as:
a) users will get warnings about things outside of their control
b) the test output is full of junk-warnings
Off the top of my head, I'd imagine doing a find and replace all on external_link( to update to shinyGovstyle::external_link( in functions like support_panel() would make the deprecation process a lot cleaner.

[cjrace]

Good point on the reuse of this in other functions - I hadn't clocked that at all, sorry!

Will move the PR to draft and update accordingly, you're right that should be handled more neatly (and also that we should take our own advice and move to using shinyGovstyle within the package itself too).

[cjrace]

That should be sorted now @rmbielby, thank you for catching that!

I'd taken the approach of just moving our internal calls straight over to shinyGovstyle. Though there is also the always argument that we can use instead or in other instances instead, that will toggle whether only direct calls get a warning or not.

I've also marked the bad_link_text data object as deprecated too for completeness, as I spotted that in my searching around the package for any references. I copied the approach we took in dfeR for wd_pcon_lad_la_rgn_ctry for that and it's a little simpler than doing it for a function.

Air did a bit of tidying on some of those new files (e.g. the bad links list) too, just in case it looks like more changes than you're expecting.