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

Display permalinks on hover #95

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Display permalinks on hover #95

merged 1 commit into from
Oct 26, 2020

Conversation

anlau
Copy link
Contributor

@anlau anlau commented Oct 1, 2020

Closes issue #37. It's a change in behavior.

Result:

image

Removed css section "before" had quite an evolution in mkdocs theme. I took the final version (see PR 1931).

Note styles used for "headerlink" class. "font-family" and "font-weight" are the same as on "fa" class.

@chrissimpkins
Copy link
Owner

Thoughts about the anchor icon? TBH, I'm not a huge fan. I do like the on hover behavior and ability to have a header perma link.

@anlau
Copy link
Contributor Author

anlau commented Oct 3, 2020

Hehe, no problem! It can be changed in mkdocs.yml:

markdown_extensions:
    - toc:
        permalink: # Some Font Awesome icon or True

@chrissimpkins
Copy link
Owner

Is it possible to change the default that is used in this project without the yml configuration edit by a user? Do you know if we can add it to the https://github.com/chrissimpkins/cinder/blob/master/cinder/mkdocs_theme.yml?

@anlau
Copy link
Contributor Author

anlau commented Oct 6, 2020

I'm not sure it's possible. Theme configurations are loaded under "theme" (config['theme']). markdown_extensions are a level above, at the root (config['markdown_extensions']).

@mmatyas
Copy link
Contributor

mmatyas commented Oct 6, 2020

Would it be possible for the permalink option to support both characters and a FA icons? Personally I'd prefer keeping the default ¶-sign, a literal anchor is not particularly meaningful in non-English languages.

@anlau
Copy link
Contributor Author

anlau commented Oct 8, 2020

True (or any yaml equivalent) displays ¶-sign. Otherwise, set character is used. If headerlink class uses Font Awesome then an icon is displayed. It raises a good point. Yes it both options are available but it's hard to accommodate both with the same css. I think we have to commit on one (and maybe document it).

I did some more tests:

  1. permalink: True
.headerlink {
    display: none;
    padding-left: .5em;
}

image

permalink can't be changed to a FA icon.

  1. permalink:  # Link icon, maybe this is a better one ;)
.headerlink {
    font-family: "Font Awesome 5 Free";
    font-size: 14px;
    font-weight: 900;
    display: none;
    padding-left: .5em;
}

image

Note that content wiggles when h6 is hovered.

If permalink is set to True then ¶-signs look small.

image

  1. Maybe we could use option 2 but increase font-size; with 18px it wiggles at h5. Also, it doesn't look as good as option 1 and FA icons seem a bit big on headers besides h2.

image

In any case, one user can modify the css with "extra_css" configuration.

Let me know what you think.

@chrissimpkins
Copy link
Owner

chrissimpkins commented Oct 8, 2020

My 2 cents is that the link icon more intuitively indicates that this is a link to the header. I'm not opposed to the pilcrow symbol. It is likely used frequently enough out there for this purpose.

As I mentioned before, I would really prefer not to show an anchor icon by default and force users to explicitly edit configuration files to use something else. If the markdown_extensions > toc > permalink setting is not present in a user configuration file, does this mean that they get no permalinks? Or is the new behavior always on hover permalinks with anchor icon default that can be modified to a different icon if you specify the permalink setting (True or icon name) in the yaml config file?

@anlau
Copy link
Contributor Author

anlau commented Oct 9, 2020

Configuration is on the user side. Without permalink, headerlink a is not generated. This is the default for a fresh install (mkdocs new myproject). One user can then set permalink to true to use the default permalink character (¶). Or any other character (or a FA icon if it's supported by his theme).

I'm not sure if I prefer the icon or the symbol.

@chrissimpkins
Copy link
Owner

Configuration is on the user side. Without permalink, headerlink a is not generated. This is the default for a fresh install (mkdocs new myproject). One user can then set permalink to true to use the default permalink character (¶). Or any other character (or a FA icon if it's supported by his theme).

I'm not sure if I prefer the icon or of the symbol.

This approach sounds good to me. Willing to merge this.

@chrissimpkins
Copy link
Owner

@mmatyas any thoughts about the two approaches that Antoine posted above?

@mmatyas
Copy link
Contributor

mmatyas commented Oct 9, 2020

Sure, that sounds fine to me as well!

@anlau
Copy link
Contributor Author

anlau commented Oct 12, 2020

So, which one should I go for? Style-wise I have a small preference for the symbol.

If it can help, MkDocs default themes use a Font Awesome icon and Material theme a symbol.

On a side note, after this PR is completed we should update MkDocs themes page (update screenshot and add badge: https://img.shields.io/pypi/dm/mkdocs-cinder).

@chrissimpkins
Copy link
Owner

So, which one should I go for? Style-wise I have a small preference for the symbol.

If it can help, MkDocs default themes use a Font Awesome icon and Material theme a symbol.

On a side note, after this PR is completed we should update MkDocs themes page (update screenshot and add badge: https://img.shields.io/pypi/dm/mkdocs-cinder).

Which one is the symbol? The pilcrow?

@anlau
Copy link
Contributor Author

anlau commented Oct 13, 2020

Yes, the pilcrow. Sorry if it was not clear.

@chrissimpkins
Copy link
Owner

Sounds good to me. Let's go with that approach.

@chrissimpkins
Copy link
Owner

All set to go here after 92bedf9 @anlau?

@chrissimpkins
Copy link
Owner

This looks great Antoine. I am updating the documentation with these changes but I can't figure out how to activate the Font Awesome icons. When I try to enter a FA icon name, the hover link shows me the string that I enter, not an icon.

@chrissimpkins
Copy link
Owner

Reviewing the conversation, it looks like you indicated that with this approach we cannot use FA icons. Only the pilcrow. I think that I misunderstood. Will merge and push this release with documentation of the pilcrow only. If it is possible to use FA icons too, please let me know and we will update the docs.

@chrissimpkins chrissimpkins merged commit 92bedf9 into chrissimpkins:master Oct 26, 2020
@chrissimpkins
Copy link
Owner

Released in v1.1.0

https://github.com/chrissimpkins/cinder/releases/tag/v1.1.0

Thank you very much for the changes here Antoine!

@chrissimpkins
Copy link
Owner

I activated this on our documentation site too btw 👍

@anlau
Copy link
Contributor Author

anlau commented Oct 27, 2020

Reviewing the conversation, it looks like you indicated that with this approach we cannot use FA icons. Only the pilcrow. I think that I misunderstood. Will merge and push this release with documentation of the pilcrow only. If it is possible to use FA icons too, please let me know and we will update the docs.

Indeed, there's no font-family: "Font Awesome 5 Free", so you can't use FA icons without additional modifications (ex. using an extra_css). Like I said above, it's hard to find css properties which render nicely both cases (see option 3). But you can still use an UTF-8 character of your choice. Maybe we could do something to support both correctly.

@chrissimpkins
Copy link
Owner

I'm happy with where we are now! I was just trying to figure out how to document FA icon use and kept getting the string that I entered into the configuration file as the permalink 😀

@anlau anlau deleted the permalinks-on-hover branch October 28, 2020 22:48
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