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

Closes #35 moved presentation archive to gt #40

Merged
merged 6 commits into from
Jan 29, 2024

Conversation

manciniedoardo
Copy link
Collaborator

Closes #35

@manciniedoardo
Copy link
Collaborator Author

@ddsjoberg I think I've successfully moved to using gt, only thing I can't figure out is why my columns are not bold and why their width is not being controlled... any ideas?

Also I guess I haven't looked at using icons yet but can do that later.

Copy link

github-actions bot commented Jan 24, 2024

Unit Tests Summary

1 files  1 suites   25s ⏱️
3 tests 3 ✅ 0 💤 0 ❌
7 runs  7 ✅ 0 💤 0 ❌

Results for commit a68cdb8.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jan 24, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
discovery 💚 $25.74$ $-12.65$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
discovery 💚 $23.83$ $-12.66$ Check_Valid_URLs

Results for commit b2dcb14

♻️ This comment has been updated with latest results.

@ddsjoberg
Copy link
Collaborator

ddsjoberg commented Jan 24, 2024

hey hey @manciniedoardo ! thanks for the PR, the table looks great!

I see that you made the presentation history a data set saved with the package. If we go that route, you'll need to add data documentation to data.R. I see you added it below the discovery data frame, but it needs to be preceded by roxygen comments for that data frame to avoid this note i the r cmd check

image

Since there are only 10 rows in the data frame, I don't think we need to make it an interactive table. If you remove the interactivity, the column widths will be as you've specified.

Thanks!

@ddsjoberg
Copy link
Collaborator

Looks almost perfect! I see the roxygen2 comments for the data set docs, but it looks like you'll need to re-document and push to get the Rd file. There is also a small typo in the data set documentation "fo" instead of "of"

Copy link
Collaborator

@ddsjoberg ddsjoberg left a comment

Choose a reason for hiding this comment

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

Looks great!

@ddsjoberg ddsjoberg merged commit 61e4e02 into main Jan 29, 2024
7 checks passed
@ddsjoberg ddsjoberg deleted the 35_presentation_archive_glowup branch January 29, 2024 16:10
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.

Presentation Archive Glowup
2 participants