Skip to content

Fixes length issues, capital alignment issues, author name overlap issue #5

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

Open
anjannair wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fixes length issues, capital alignment issues, author name overlap issue #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a69cbe3
Fixes length issues, capital alignment issues, author name overlap issue
anjannair May 9, 2022
f0a5c8e
Checking for all caps
anjannair May 9, 2022
59d1510
Added the text-transform removed JS transform
anjannair May 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion bookshelf.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,15 @@
color: gold;
writing-mode: vertical-rl;
text-orientation: mixed;
/* Adding a bottom padding so that it does not mess with the author name */
padding-bottom: 15px;
text-transform: capitalize;
}

.spine-author {
position: absolute;
color: goldenrod;
bottom: 0px;
bottom: 0px;
left: 20%; /* no idea why 20% centers it */
}

Expand Down
8 changes: 7 additions & 1 deletion bookshelf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ function randomChoice(array) {
let spines = Object.values(document.getElementsByClassName("spine"));
let covers = Object.values(document.getElementsByClassName("cover"));
let tops = Object.values(document.getElementsByClassName("top"));
let titles = Object.values(document.getElementsByClassName("spine-title"));

let availablePatterns = ["argyle", "tartan"]; // we could probably get these programatically
let availablePatterns = ["argyle", "tartan"]; // we could probably get these programmatically
let availableColours = [
"maroon",
"darkgreen",
Expand All @@ -39,4 +40,9 @@ spines.map(function (s, i) {
covers[i].style.top = `${280 - randomHeight}px`;

tops[i].style.top = `${280 - randomHeight}px`;

// 75 characters above spoils the alignment
if (titles[i].innerHTML.length > 75) {
titles[i].innerHTML = titles[i].innerHTML.substring(0, 73) + "...";
}
Comment on lines +44 to +47
Copy link
Owner

@petargyurov petargyurov May 9, 2022

Choose a reason for hiding this comment

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

Isn't this more elegantly achieved using text-overflow: ellipsis; (and possibly some other CSS properties) ?

Copy link
Author

@anjannair anjannair May 9, 2022

Choose a reason for hiding this comment

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

text-overflow: ellipsis is a good property but I could not get it to work even with other properties. However I found this in the MDN docs

The text-overflow property only affects content that is overflowing a block container element in its inline progression direction (not text overflowing at the bottom of a box, for example).

Copy link
Author

@anjannair anjannair May 9, 2022

Choose a reason for hiding this comment

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

Also just a question - What is the problem with JavaScript? (A generalized question for both the comments)

Copy link
Owner

@petargyurov petargyurov May 9, 2022

Choose a reason for hiding this comment

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

No problem with JS, I think it's more about just using the right tool for the job.

I'd like to have a go at seeing if text-overflowcould work, hope that's ok. Otherwise this should be an okay way to deal with it. I appreciate the help 👍

Copy link
Owner

@petargyurov petargyurov May 15, 2022

Choose a reason for hiding this comment

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

Hey, I just had a go at trying to fix this and yeah, it's very difficult to get it perfect using just CSS. I got close but not quite. The issue is dealing not just with long titles but specifically long words.

Anyway, your solution works well, however your chosen values didn't work for me. I had to reduce the character length to 40 in the JS. I experimented with various different titles with long words, many short words, etc... Are you able to confirm?

Copy link
Author

@anjannair anjannair May 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

40 instead fits perfectly for me and my chosen values goes out of bounds
Browser - Firefox v100
Size - 1920x968 px

Copy link
Author

@anjannair anjannair May 15, 2022

Choose a reason for hiding this comment

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

image

});