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

Prettier Indentation of TOC Items Containing Line Breaks #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Prettier Indentation of TOC Items Containing Line Breaks #22

wants to merge 1 commit into from

Conversation

unixtam
Copy link
Contributor

@unixtam unixtam commented Mar 6, 2018

Before:
eztoc_before

After:
eztoc_after

As a side effect the numbers are no longer linked. I also set them a little further apart and removed the last dot. I think it looks cleaner that way. Just a suggestion.

PS: Your screen.min.css looks like it needs updating even without merging this PR.

@unixtam
Copy link
Contributor Author

unixtam commented Mar 6, 2018

BTW, it's entirely possible to have the numbers linked as well and still indent nicely, if you prefer. However, there would be a gap in the underline text decoration (as on Wikipedia), and box-shadow underlining (as e.g. used by the TwentySeventeen theme shown in the screenshots) would break due to making the anchor a block element.

It would look like this:
eztoc_after_alt
The third item shows the gap in text decoration, the others show the broken box-shadow underline. That's why I prefer unlinked counters as in the above screenshots, where there is no gap and the underlines don't break.

@shazahm1
Copy link
Contributor

shazahm1 commented Mar 6, 2018

I am not against improving the styling, but here are my thoughts on what is presented:

  • I definitely do not like the last screenshot (in your second comment). It looks odd and inconsistent.
  • I am fine with removing the trailing period only as long as there is better differentiation between the numeral and the title text. The "after" screenshot, even with the gap it still appears as if the numeral is part of the title. The trailing period gives the necessary differentiation.

The most important item to me... I use the widget on my own site. Any styling changes must not break the widget line highlighting as shown on this page:

https://connections-pro.com/documentation/settings/

And before approving any CSS changes I would need to see a screenshot of all counter variations.

Thanks for your work on improving ezTOC!!!

@unixtam
Copy link
Contributor Author

unixtam commented Mar 6, 2018

I definitely do not like the last screenshot (in your second comment)

Neither do I. That was just to demonstrate why I didn't link the counters/numerals.

better differentiation between the numeral and the title text

On my site the numeral is colored differently from the title text, but I didn't want to make too extensive changes to your defaults.

Thanks for your work on improving ezTOC!!!

No problem. I'm really just scratching some personal itches. When I reached the limit of what TOC+ could do and development seemed stale, I looked into alternatives. EasyTOC is a lovely fork and I'm glad I can contribute a little with my PRs. So thank you for your continuous development.

As for this PR in particular, on my site I have disabled the default stylesheet and use my own CSS instead. I just wanted to share the code I used for prettier indentation on small screens because I tried other ways of indentation before and this one is almost deceptively simple. But maybe it's a change that isn't needed right now, so I'll go ahead and close this PR. If someone asks for improved indentation on small screens, you'll where to find some inspiration.

@unixtam unixtam closed this Mar 6, 2018
@shazahm1
Copy link
Contributor

shazahm1 commented Mar 6, 2018

Thanks! I'm actually going to keep this opened otherwise it sort of becomes hidden.

@shazahm1 shazahm1 reopened this Mar 6, 2018
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.

2 participants