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

Enhancement: Refactor Visualisation Tab #136

Merged
merged 11 commits into from
Dec 19, 2024

Conversation

js3110
Copy link
Collaborator

@js3110 js3110 commented Nov 22, 2024

Issue

Closes #127

Description

With the introduction of the TLGs tab, the Outputs section now is no longer required for reporting but rather for enhanced data visualisation. We want to change the layout so it is better for visualisation.

Definition of Done

  • rename outputs to visualisation tab
  • Modify layout to make for better exploratory analysis- bigger plots

How to test

Check visualisation tab layout

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented

Notes to reviewer

The plan is to reformat this section using bslib, which will then need to be implemented across the rest of the app. Refactoring of the Data section is being covered by #140

@js3110 js3110 linked an issue Nov 22, 2024 that may be closed by this pull request
2 tasks
@js3110 js3110 marked this pull request as ready for review December 13, 2024 09:17
@js3110 js3110 mentioned this pull request Dec 13, 2024
7 tasks
Copy link
Collaborator

@m-kolomanski m-kolomanski left a comment

Choose a reason for hiding this comment

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

Hey, good job in general, must have taken a lot of effort to get that together! The UI looks quite nice, my only suggestion would be to maybe try and work a bit with the widths of different elements (the drag-and-drop, buttons etc) to make it a bit more tight.

I also have some points of discussion, mainly in terms of code style and format.

inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
@js3110 js3110 requested a review from m-kolomanski December 16, 2024 15:17
Copy link
Collaborator

@m-kolomanski m-kolomanski left a comment

Choose a reason for hiding this comment

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

I have approved the PR, code looks good, great work! I have left some minor nitpicks, but I consider them non-blocking.

Might be also worth adding some logs to better trace what is going on in the application.

inst/shiny/modules/column_mapping.R Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
inst/shiny/modules/tab_visuals.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@Gero1999 Gero1999 left a comment

Choose a reason for hiding this comment

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

lgtm

@js3110 js3110 merged commit fb38102 into main Dec 19, 2024
9 checks passed
@js3110 js3110 deleted the enhancement/improve-visualisation-section branch December 19, 2024 08:56
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.

Enhancement: Improve Data Visualisation section
3 participants