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.

Sign up for GitHub

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

Jump to bottom

tmh css #272

Merged
merged 13 commits into from
Jul 29, 2022
Merged

tmh css #272

merged 13 commits into from
Jul 29, 2022

Conversation

Polkas
Copy link
Contributor

@Polkas Polkas commented Jul 25, 2022 •
edited
Loading

linked to https://github.com/insightsengineering/coredev-tasks/issues/225

  • div + display="inline" == span
  • remove br() tag as
    would have different result in different browsers.
  • consistent loading of css with tagList(include_css, main_tag)
  • add style at the head not body

@Polkas Polkas marked this pull request as ready for review July 26, 2022 09:47
@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2022 •
edited
Loading

Code Coverage Summary

Filename              Stmts    Miss  Cover    Missing
------------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------
R/adtteSpec.R           157      11  92.99%   308-312, 358-362, 364
R/assaySpec.R            44      36  18.18%   102-142
R/barplot.R             162     129  20.37%   40-64, 122-248
R/boxplot.R             165     137  16.97%   41-65, 119-251
R/checkmate.R            18       9  50.00%   110-118
R/experimentSpec.R       92      58  36.96%   99, 214-285
R/forestplot.R          193     168  12.95%   60-91, 146-306
R/geneSpec.R            255     136  46.67%   153-168, 297-448
R/km.R                  182     151  17.03%   63-92, 152-297
R/pca.R                 344     261  24.13%   35-55, 162-449
R/quality.R             291     221  24.05%   18-109, 202-421
R/sampleVarSpec.R       221     100  54.75%   296-302, 314-321, 323, 331-342, 344-345, 347, 350, 358-362, 364-379, 384-407, 410-414, 416, 423-433, 435-436, 444, 489-506
R/scatterplot.R         160     130  18.75%   40-64, 121-248
R/utils.R                16       5  68.75%   74-78
R/volcanoplot.R         187     157  16.04%   35-56, 110-271
R/zzz.R                   1       1  0.00%    2
TOTAL                  2488    1710  31.27%

Results for commit: b2c9d9e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions
Copy link
Contributor

github-actions bot commented Jul 26, 2022 •
edited
Loading

Unit Tests Summary

  1 files  15 suites   17s ⏱️
58 tests 36 ✔️ 22 💤 0 ❌
85 runs  63 ✔️ 22 💤 0 ❌

Results for commit 94536be.

♻️ This comment has been updated with latest results.

@Polkas Polkas removed the blocked label Jul 28, 2022
@gogonzo gogonzo self-assigned this Jul 29, 2022
gogonzo
gogonzo approved these changes Jul 29, 2022
View reviewed changes
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

LGTM

nikolas-burkoff
nikolas-burkoff reviewed Jul 29, 2022
View reviewed changes
R/pca.R
width = 12,
div(style = "height:20px;"),
plotOutput(ns("plot_cor")),
br(), br(), br(),
Copy link
Contributor

Choose a reason for hiding this comment

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

these not needed any more

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what you mean

Copy link
Contributor

Choose a reason for hiding this comment

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

The br(), br(), br() is missing now?
image

Is that OK?

Copy link
Contributor Author

@Polkas Polkas Jul 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

my-5 replace the upper div and the lowe br's. It is ok. y means the y axis so top and bottom margin

Polkas
Polkas commented Jul 29, 2022
View reviewed changes
R/geneSpec.R Outdated Show resolved Hide resolved
@Polkas Polkas merged commit 0e6dce9 into main Jul 29, 2022
@Polkas Polkas deleted the core_225_css@main branch July 29, 2022 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikolas-burkoff nikolas-burkoff nikolas-burkoff left review comments

@gogonzo gogonzo gogonzo approved these changes

Assignees

@gogonzo gogonzo

Labels
core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Polkas @gogonzo @nikolas-burkoff