-
Notifications
You must be signed in to change notification settings - Fork 16
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
fix: new semantic HTML selectors #887
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #887 +/- ##
=======================================
Coverage 47.96% 47.96%
=======================================
Files 4 4
Lines 467 467
=======================================
Hits 224 224
Misses 243 243
Continue to review full report at Codecov.
|
@@ -219,13 +219,7 @@ | |||
} | |||
|
|||
@if $type == 'prince' { | |||
h3.front-matter-number, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed anymore https://www.princexml.com/doc/pdf-bookmarks/#bookmark-levels because we are not using a title tag
@@ -167,7 +167,7 @@ h1.front-matter-title { | |||
} | |||
|
|||
// PART TITLE COLOR | |||
h3.part-number { | |||
p.part-number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be better if we removed element selectors altogether and just targeted elements with the .part-number class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we discussed that on Friday I wasn't sure if the .part-number
exists in other contexts but definitely I can look into that
@SteelWagstaff I sent the agreed changes in the morning and I used a new commit message using the conventional commits strategy |
h1.back-matter-title + h2.chapter-subtitle, | ||
.short-title + h2.chapter-subtitle { | ||
h1.back-matter-title + .chapter-subtitle, | ||
.short-title + p.chapter-subtitle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove h1 and p references here?
5a29d2e
to
2de84a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h1.back-matter-title + h2.chapter-author, | ||
.short-title + h2.chapter-author { | ||
h1.back-matter-title + .chapter-author, | ||
.short-title + p.chapter-author { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove h1 and p references here?
margin-top: if-map-get($section-title-author-spacing, $type); | ||
} | ||
|
||
h2.chapter-subtitle + h2.chapter-author { | ||
.chapter-subtitle + .chapter-author { | ||
margin-top: if-map-get($section-subtitle-author-spacing, $type); | ||
} | ||
|
||
h1.back-matter-title:last-child { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove h1 reference here?
@@ -114,25 +114,25 @@ | |||
word-spacing: if-map-get($section-author-word-spacing, $type); | |||
} | |||
|
|||
h2.chapter-title + h2.chapter-subtitle, | |||
.short-title + h2.chapter-subtitle { | |||
h2.chapter-title + .chapter-subtitle, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove h2 reference here?
@@ -177,7 +177,7 @@ blockquote.aphorism { | |||
margin-bottom: 0; | |||
} | |||
|
|||
h3.chapter-number { | |||
p.chapter-number { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove p reference here?
@@ -78,25 +78,25 @@ | |||
word-spacing: if-map-get($section-author-word-spacing, $type); | |||
} | |||
|
|||
h1.front-matter-title + h2.chapter-subtitle, | |||
.short-title + h2.chapter-subtitle { | |||
h1.front-matter-title + .chapter-subtitle, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove h1 reference here?
margin-top: if-map-get($section-title-subtitle-spacing, $type); | ||
} | ||
|
||
h1.front-matter-title + h2.chapter-author, | ||
.short-title + h2.chapter-author { | ||
h1.front-matter-title + .chapter-author, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove h1 reference here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Differences between digital PDF
- Links including author names are not styled with the same link color as other links in the ToC:
- The introduction is not moved into its expected order -- should appear after the foreward. See https://guide.pressbooks.com/chapter/create-and-edit-front-matter/#chapter-179-section-5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved for merging
This PR closes pressbooks/pressbooks#2452
How to test.
Export a book with different formats and you should see the same styling as the integrations/production networks.