feat: move plot/table modules, modal, dropdown and navigation in from OPG - #28
Open
ESCRI11 wants to merge 9 commits into
Open
feat: move plot/table modules, modal, dropdown and navigation in from OPG#28ESCRI11 wants to merge 9 commits into
ESCRI11 wants to merge 9 commits into
Conversation
… OPG PlotModule and TableModule lived in omicsplayground, which meant a standalone BigOmics app had to drag the whole thing in to get a plot card. They move here along with what they need to render: DropdownMenu, modalUI/modalTrigger, the server-side navigation API, and the CSS/JS for all of it. The modules were wired into Omics Playground globals (i18n, watermarking, the plot editor, download telemetry, the colour theme). Those are now looked up as options via bd_hook() and fall back to a no-op, so bigdash works on its own and OPG registers its own implementations on startup. FILES, opt$WATERMARK, OPG and LOADEDPGX no longer appear here at all. Also folded in the styling OPG kept under the comment "THESE CHANGES SHOULD EVENTUALLY BE MOVED TO BIGDASH", merged into the matching bigdash modules rather than appended as a second layer. inst/examples/modules.R is a runnable demo, and the smoke test behind SMOKE=TRUE. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
inst/examples/standalone.R is a full BigOmics-looking dashboard built from bigdash alone: navbar, sidebar navigation, settings drawer, two PlotModules (plotly and base R) and a TableModule, over base R datasets. No opt, no FILES, no pgx, no sourced components/ tree. Also the render-only self-check, behind SMOKE=TRUE. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PlotModule/TableModule size their header icon buttons off --bs-btn-padding-x/y, which OPG sets in _all.scss. That file stayed behind, so bigdash fell back to Bootstrap's defaults: the header grew 56px instead of 52px and the buttons spread out at 66x45 instead of 42x41. Verified by diffing computed styles in the live page with and without OPG's pre-migration stylesheet injected. This rule alone accounted for the whole difference, and 19 probes across header, title, info popover, caption, footer, table header, sidebar, settings, navbar and tick are now identical either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bootstrap's 16px default rendered the whole dashboard ~14% larger than Omics Playground, which sets 14px in its own sheet. Everything with an explicit size matched already, which is why the earlier probe missed it: the drift was in everything that inherits. Compared against a live Omics Playground board (make board.example). Body font was the last remaining difference; header height, title, icon buttons and caption were already identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The module header buttons are 19x19 in Omics Playground because shinyWidgets ships `.btn.btn-circle-xs` with explicit width/height/font-size. OPG's own sheet only sets their colours, so there was nothing to move and a standalone app fell back to full-size buttons: 42x41, pushing the header from 33px to 52px. Declared here with shinyWidgets' values rather than taking the dependency for six declarations; identical either way when both are loaded. Found by running the real app (dev/run_app.R) rather than the board.launch harness, which does not load shinyWidgets and so hid the difference. Verified against it: header 33px, padding 5px 6px 5px 12px, button 19x19, title 16px/h24, body 14px, caption 13px all match exactly. A scan of every third-party stylesheet for module selectors turns up nothing else outstanding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A set-difference of every CSS declaration removed from OPG against everything bigdash gained found two that were lost rather than moved: .sidebar-menu-item padding: 0 0.4rem 0 0.4rem (bigdash kept its own padding-left: 1rem) .settings-expanded width: 12rem (bigdash had only min-width) Both are sidebar/settings, which is why the card-focused probes missed them. Confirmed by running the pre- and post-migration app side by side: the .sidebar-menu-item padding was the one metric that differed, and matches now. Coverage is 102 of 105 removed declarations; the other 3 are verified equivalents (#004CA7 vs #004ca7, black vs #000, and the deliberate img/mascotte-sc.png path change). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rsion Three findings from an independent review of the migration. send_nav() replaced shiny:::validate_session_object with a check for "ShinySession", but inside moduleServer the session is a bare "session_proxy" which does not inherit from it. Every bigdash.* navigation call made from within a module therefore threw instead of navigating - including the Welcome page's "Upload new data" button and the jump to load-tab after a compute finishes. Now matches shiny's own accepted class list. Verified: a module session_proxy is accepted, a non-session is still rejected. Version had gone backwards, 0.0.1 -> 0.0.0.9005, which sorts LOWER; installers with upgrade="never" or a warm Docker layer would silently keep the old package while OPG has already deleted its copies. Bumped to 0.0.2. tools:: and utils:: were used but undeclared (R CMD check WARNING). Also moved the editor_content hook lookup above the editor button so that disabling the editor actually suppresses the button, rather than leaving one that opens an empty modal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nesting it under `html` raised it from (0,1,0) to (0,1,1). `.tick` is also what D3 and plotly emit for axis ticks, so an `opacity: 0` rule with that reach should not win more selectors than it did in Omics Playground. Now byte-identical to the original _tick.scss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Baseline (origin/master): 1 ERROR, 3 WARNINGs, 1 NOTE This branch: 1 ERROR, 1 WARNING, 1 NOTE Fixed here: - documented every PlotModuleUI/PlotModuleServer argument; the roxygen I wrote used a `@param ...` catch-all but neither function takes `...` - declared the grDevices/graphics/utils functions the moved code calls unqualified (png, pdf, dev.off, par, mtext, head, tail, write.csv) - .Rbuildignore now excludes the untracked react/ sibling, whose node_modules were being swept into the tarball and tripping the executable-files WARNING - moved a \dontrun out of @details into @examples, where checkRd accepts it - bs_alert reached for its own package with bigdash:::make_id() Left alone, both pre-existing and identical on origin/master: the hover_dropdown example ERROR, and undocumented arguments on bs_alert/navbar/navbarDropdown. The remaining NOTE is only that four optional Suggests are not installed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion PR: bigomics/omicsplayground# — merge this one first.
What this does
Moves
PlotModule,TableModuleand their supporting UI out of Omics Playground and into bigdash, so a standalone BigOmics app can use them without dragging in OPG.Arriving here:
PlotModuleUI/PlotModuleServer,plotlyExport,visPrintui-PlotModule.RTableModuleUI/TableModuleServer,CardUI,ui.DataTable,trunc_display_row,color_from_middleui-TableModule2.R,ui-colors.RDropdownMenuui-DropDownMenu.RmodalUI/modalTriggerui-modalUI.Rbigdash.*navigation APIui-bigdashplus.Rscss/,www/temp.js+ 3 helpersAlso folds in the styling OPG kept under the comment "THESE CHANGES SHOULD EVENTUALLY BE MOVED TO BIGDASH", merged into the matching bigdash partials rather than appended as a second layer.
Decoupling from OPG
The modules reached directly for
opt$WATERMARK,FILES,tspan,getEditorContent,record_plot_download,addSettings,addWatermark.*andCOLOR_THEME_MAPPING. Those are now looked up as options viabd_hook(), each with a no-op fallback, so bigdash works standalone and OPG registers its own implementations at startup. None of those OPG globals appear in this package any more (verified withcodetools::findGlobals).inst/examples/standalone.Ris a runnable demo — full dashboard, two PlotModules and a TableModule over base R datasets, no OPG present.SMOKE=TRUE Rscript inst/examples/standalone.Rruns it as a render-only self-check.Not a pure move
Three CSS declarations are added, needed for a standalone app to match OPG:
.btnpadding vars andhtml body { font-size: 14px }— moved from OPG's_all.scss.btn.btn-circle-xssizing — this was never OPG's; it comes from shinyWidgets, which OPG happens to load. Declared here (same values) rather than taking the dependency for six declarations.Verification
#004CA7/#004ca7,black/#000, and the deliberateimg/mascotte-sc.pngpath). Zero declarations added to OPG.console.logdropped frommodalUI, thedropdown-helper.jstag now in this bundle, and bslib's random accordion ids (proved nondeterministic by rendering the same tree twice).R CMD checkvsorigin/master: WARNINGs 3 → 1, ERROR and NOTE unchanged. The remaining ERROR (hover_dropdownexample) and WARNING (bs_alert/navbar/navbarDropdownargs) are pre-existing and identical on master.Known behavioural deltas
dbg()→message()in two download handlers — log formatting only.input$get_pdf_settingsis nowisTRUE(...); a NULL used to error and abort the PDF download.console.log('Modal event:'…)removed frommodalUI(track_open = TRUE).editor = TRUEwithout a registeredbigdash.editor_contenthook now warns and drops the button instead of rendering one that opens an empty modal.🔎 What we're asking reviewers to confirm
The one thing that matters: Omics Playground must look and behave exactly as it did before. This is a code move, not a redesign. Please try to prove that wrong.
Highest-value checks, roughly in order:
edgy.make run, load the example dataset, and walk the boards you know best. Every plot/table card should be pixel-identical: header height, the info/options/download/zoom buttons, title, caption, the fullscreen zoom modal, downloads (PNG/PDF/CSV), and the plot editor.bigdash.*call made from inside a Shiny module — the Welcome page's upload buttons, and the jump toload-tabafter a compute finishes. Fixed and tested, but please exercise those paths: Welcome → "Load new data", and a full upload → compute → load cycle.WATERMARK = TRUEand confirm the stamp and the appended settings table.Differences inside plots (colour palettes) are expected and out of scope — the palette helpers stayed in OPG. It's the box around the plot that this PR moves.
No Omics Playground anywhere. Copy-paste into R — this exact script was run and screenshotted; its card chrome measures identical to OPG's (header 33px, buttons 19×19, body font 14px).
You get the full BigOmics card: info popover, download menu, fullscreen zoom, caption footer, sidebar navigation and a settings drawer driving the plot — from
library(bigdash)alone.For a fuller demo (plotly + base + a TableModule, navigation API buttons):
Rscript inst/examples/standalone.R.