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

refactor carnival tables #529

Merged
merged 32 commits into from
Aug 22, 2024
Merged

refactor carnival tables #529

merged 32 commits into from
Aug 22, 2024

Conversation

kswanson33
Copy link
Contributor

@kswanson33 kswanson33 commented Jun 25, 2024

Issue: 5494

Screenshot 2024-07-09 at 8 23 28 AM

@jbwilson8 jbwilson8 marked this pull request as ready for review July 12, 2024 12:37
Copy link
Contributor Author

@kswanson33 kswanson33 left a comment

Choose a reason for hiding this comment

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

so many styles refactored! 🎉

overall, needs another pass to look at some issues with selectors and using color/font variables. i wonder if a point of confusion comes from the sass docs on nesting selectors ... the SCSS and Sass tabs show similar syntax, but Sass doesn't use {}. we are using SCSS -- be careful not to mix the two syntaxes, use {} consistently

styles/lib/carnival/_theme.scss Outdated Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
@jbwilson8 jbwilson8 force-pushed the nursing-internal-tables branch from c660816 to e5e5705 Compare July 22, 2024 16:08
@jbwilson8 jbwilson8 force-pushed the nursing-internal-tables branch from e5e5705 to 10f6e25 Compare July 24, 2024 13:46
styles/lib/carnival/_theme.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
@jbwilson8
Copy link
Contributor

Latest view of table header in Clinical Nursing:

Screenshot 2024-08-08 at 10 50 54 AM

Copy link
Contributor Author

@kswanson33 kswanson33 left a comment

Choose a reason for hiding this comment

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

very close, i can see why it might be pretty difficult to identify the inconsistencies in the output ... need a better solution for this

update: https://github.com/openstax/cnx-recipes/issues/5596

styles/output/nursing-internal-pdf.css Show resolved Hide resolved
styles/output/nursing-internal-pdf.css Outdated Show resolved Hide resolved
styles/lib/carnival/features/_tables.scss Outdated Show resolved Hide resolved
@jbwilson8 jbwilson8 force-pushed the nursing-internal-tables branch from 80709ab to ad64974 Compare August 9, 2024 15:42
@sparksam sparksam self-requested a review August 20, 2024 15:49
Copy link
Contributor

@sparksam sparksam left a comment

Choose a reason for hiding this comment

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

The code looks good to me.

@jbwilson8 jbwilson8 merged commit ca5f012 into main Aug 22, 2024
2 checks passed
@jbwilson8 jbwilson8 deleted the nursing-internal-tables branch August 22, 2024 19:31
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.

3 participants