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

shinyMobile 2.0.0 #256

Merged
merged 396 commits into from
May 25, 2024
Merged

shinyMobile 2.0.0 #256

merged 396 commits into from
May 25, 2024

Conversation

DivadNojnarg
Copy link
Member

@DivadNojnarg DivadNojnarg commented Mar 7, 2024

Poke @hypebright.

TO DO/roadmap:

  • Review anything broken with new update. Right now, most elements don't show (I haven't checked further, just an observation).
  • Check if some code elements are outdated/deprecated.
  • Remove all deprecated functions (they are deprecated for a long time already).
  • Rework split layout: separation between sidebar and body
  • Check that the doc does not point to https://v5.framework7.io/docs anymore (old v5 doc).
  • validateF7Input: does validation still work?
  • Store all input instances like widget (currently there are under the input binding instances but they could be in the global app store to leverage updateF7Entity) --> THIS DOES NOT WORK SMOOTHLY FOR INPUTS (some inputs need modified attributes in addition to the config json, which is too complex to cover, too many edge cases). So for now, we won't support inputs.
  • Use new version number (2.0.0?)
  • Check CRAN notes
  • Implement router layout: transitions between page, ... -> We have something, it now needs a vignette.
  • Update existing vignettes
  • Add router vignette
  • Add socket reconnect vignette (from example in inst/tests folder)
  • Add vignette on inputs, putting them in lists, and forms
  • Look into filled option in f7Page
  • Update (or add) all shinylive examples
    • index [pending Veerle]
    • shinyMobile
    • shinyMobile-tools
    • inputs-layout
    • update-app
  • Test for router layout? --> Depends on something not on CRAN. We can't really test it for now.
  • revdeps check
  • Remove old js files like app-old.js
  • Add vignette on using updateF7App and updateF7Entity
  • Update to v8.3.3 :)
  • Rework gallery
  • Strava app[pending David] -> Move to another repo
  • Check PWA offline page: maybe update service worker static assets path because we don't expose framework7 css and JS script directly. That said, caching shinyMobile.min.js and CSS might be enough as we import Framework7 elements in them.

Components progress:

  • Accordions
  • Action Sheet
  • Appbar: removed
  • Autocomplete: the R code would have to be revisited to have a simpler input binding like in the f7DatePicker.
  • Badge
  • Block
  • Button
  • Calendar/date picker
  • Cards
  • Checkbox/CheckboxGroup: update label does not work but probably never did. Also there's no updateCheckboxGroup (LOLOL).
  • Chips
  • Color picker
  • Contacts lists
  • Dialog
  • FAB
  • Gauge
  • Grid
  • Icons
  • Link
  • List button: this was not implemented, can do it another time.
  • List view
  • List index
  • Login screen
  • Menu: removed from Framework7
  • Menu list: this was not implemented and no big deal if we don't have it now.
  • Message bar
  • Messages
  • Navbar
  • Notifications
  • Panel
  • Photo Browser
  • Picker: updated but seems not to be as good as before...
  • Popover
  • Popup
  • Preloader
  • Progressbar
  • Pull to refresh (PTR)
  • Radio
  • Range slider
  • Search bar: maybe store the result of the search somewhere???
  • Segmented
  • Select input
  • Sheet modal
  • slider input -> Maybe a bit more test (R tag)
  • Skeleton
  • Smart select
  • Sortable list: This isn't implemented yet. No big deal if we don't make it.
  • Status bar: this isn't supported for web version: This functionality is only available when app is running under cordova/capacitor environment. So no need to add it.
  • Stepper
  • Subnavbar
  • Swipeout
  • Swiper
  • Tabs
  • Table -> Note: would be really cool to get all of the other options working like sorting, pagination. Question is how much value it adds over the use of other table packages.
  • TapHold: long press
  • Text input -> Note: updating label does not work
  • Text area input -> Note: updating label does not work
  • Timeline
  • Toast
  • Toggle
  • Toolbar
  • Tooltip
  • Treeview
  • Virtual list

NEWS.md Outdated Show resolved Hide resolved
@DivadNojnarg
Copy link
Member Author

Another change I want to introduce. Instead of having examples code in the roxygen skeleton, we'll use:

#' @example inst/examples/accordion/app.R

So that we can use this file to do testing with shinytest2

@hypebright
Copy link
Contributor

@DivadNojnarg removed functionality from v8:

  • Menu (f7Menu)
  • Appbar (f7Appbar)

We can use deprecate_warn() or deprecate_stop() here, although we can't provide an alternative. What do you suggest?

@DivadNojnarg
Copy link
Member Author

deprecate_stop() for things that have been removed and have no replacement (we can't recreate an aurora layout). deprecate_warn() for removed paramaters.

deprecate_warn("X.0.0", "foo(arg = 'has been removed from Framework7')")

so that it does not error when people use them but they won't have any effect. Also this might be good to add a lifecycle badge in the roxygen doc to say deprecated. Then on the next release, we can remove them entirely.

#' Blabla
#' 
#' @description
#' `r lifecycle::badge("deprecated")`
#' 
#' This function was deprecated because it was removed from Framework7 ...
#' @keywords internal

@DivadNojnarg
Copy link
Member Author

Example in practice:

Warning (test-f7Accordion.R:4:3): accordion
The `multiCollapse` argument of `f7Accordion()` is deprecated as of shinyMobile 1.1.0.multiCollapse has been removed from Framework7 and will be removed from shinyMobile in
  the next release.

srcjs/app.js Outdated Show resolved Hide resolved
const widgets = uiWidgets.concat(serverWidgets);

// Instantiate a widget
activateWidget = function(widget) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe rename to createWidget?

var f7MessageBarBinding = new Shiny.InputBinding();

$.extend(f7MessageBarBinding, {

initialize: function(el) {
app.messagebar.create({
this.app = getAppInstance();
this.app.messagebar.create({
Copy link
Member Author

Choose a reason for hiding this comment

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

@hypebright: if we stored the instance into this.instance =this.app.messagebar.create(...), we would not need to do all the this.app.messagebar.get. Not super fan about instance word. Maybe something shorter, except that el is already taken.

Copy link
Contributor

Choose a reason for hiding this comment

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

inst, object, obj?

},

// see updatef7Messages
setValue: function(el, value) {
var messages = this.app.messages.get(el);
var self = this;
var message = value.value;
// TO DO: does not work if TRUE
if (value.showTyping) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@hypebright I could not make this one working. You can try with this below example and put a console.log(data) in the receiveMessage messages binding method. You'll see that after sending your message through the app, the second message is empty which does not make much sense since updateF7Messages sends the correct JSON. Something with session$sendInputMessages()? Something JS side? Maybe we don't need a binding after all and just use session$sendCustomMessage + some JS code like for other widgets (gauges...). I'll continue working on the other elements.

shinyApp(
  ui = f7Page(
    title = "Messages",
    f7SingleLayout(
      navbar = f7Navbar(
        title = "Messages",
        hairline = FALSE,
        shadow = TRUE
      ),
      toolbar = f7MessageBar(inputId = "mymessagebar", placeholder = "Message"),
      # main content
      f7Messages(id = "mymessages", title = "My message")

    )
  ),
  server = function(input, output, session) {
    messageSent <- reactiveVal()
    observeEvent(input[["mymessagebar-send"]], {
      messageSent(FALSE)
      updateF7Messages(
        id = "mymessages",
        list(
         f7Message(
          text = input$mymessagebar,
          name = "David",
          type = "sent",
          header = "Message Header",
          footer = "Message Footer",
          textHeader = "Text Header",
          textFooter = "text Footer",
          avatar = "https://cdn.framework7.io/placeholder/people-100x100-7.jpg"
         )
        )
      )
      messageSent(TRUE)
    })

    observeEvent(req(messageSent()), {
      names <- c("Victor", "John")
      name <- sample(names, 1)

      updateF7Messages(
        id = "mymessages",
        showTyping = TRUE,
        list(
         f7Message(
          text = "Some message",
          name = name,
          type = "received",
          avatar = "https://cdn.framework7.io/placeholder/people-100x100-9.jpg"
         )
        )
      )
    })

  }
 )

@DivadNojnarg
Copy link
Member Author

N  checking installed package size ...
     installed size is  7.3Mb
     sub-directories of 1Mb or more:
       shinyMobile-1.0.1   4.8Mb

@hypebright
Copy link
Contributor

what happened in that last commit, this is insane...

@hypebright
Copy link
Contributor

what happened in that last commit, this is insane...

hard reset prev commit and force pushed

R/f7List.R Show resolved Hide resolved
R/f7Preloader.R Outdated
#' })
#'
#' observeEvent(input$obs, {
#' showF7Preloader(color = "red", type = "dialog", id = "loader")
Copy link
Member Author

Choose a reason for hiding this comment

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

@hypebright this is similar to {waiter} and it might feel more natural to be able to chain functions. However, I don't want to depends on R6.

label = "Choose a date",
value = Sys.Date() - 7,
label = "Choose a date and time",
value = Sys.time() - 3600*24,
Copy link
Contributor

Choose a reason for hiding this comment

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

@DivadNojnarg using things like Sys.Date() and Sys.time() will cause issues with testing. Results will always be different, so reverting back to fixed dates (and times)

Comment on lines +74 to +88
#' @param ... Item text.
#' @param id Optional id for item.
#' @param title Item title.
#' @param subtitle Item subtitle.
#' @param header Item header.
#' @param footer Item footer.
#' @param href Item external link.
#' @param media Expect \link{f7Icon} or \code{img}.
#' @param right Right content if any.
#' @param routable Works when href is not NULL. Default to FALSE. If TRUE,
#' the list item may point to another page, but we recommend using \link{f7List}
#' and \link{f7ListItem} instead. See \link{f7MultiLayout}. Can also be used in
#' combination with href = "#" to make items appear as links, but not actually
#' navigate anywhere, which is useful for custom click events.
#'
Copy link
Contributor

Choose a reason for hiding this comment

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

@DivadNojnarg I added a routable argument here, which has in strict sense no relevance for a virtual list. But it is handy to use in combination with href = "#" because based on this routable arg, we can remove the external class. I thought it was best to align with f7ListItem().

Now, in combination with the id in both list items, it is relatively user-friendly to add custom click events if desired.

What are your thoughts on this? I don't want make things too messy or too unclear, but I guess it doesn't harm much but will be very helpful in specific cases? Could support this in the lists vignette..

@DivadNojnarg DivadNojnarg marked this pull request as ready for review May 25, 2024 16:05
@DivadNojnarg
Copy link
Member Author

DivadNojnarg commented May 25, 2024

@hypebright Will need your green light for merging this into main.
I ran all the pre-release checks + submitted a PR to fix deprecation msgs to the reverse dependency.

Copy link
Contributor

@hypebright hypebright left a comment

Choose a reason for hiding this comment

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

LGTM 😎

@DivadNojnarg DivadNojnarg merged commit ab4a7dc into master May 25, 2024
7 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.

None yet

2 participants