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

no compiler warning: social links doesn't work if url parameter is not set #992

Closed
MaCXyLo opened this issue May 4, 2017 · 2 comments
Closed

Comments

@MaCXyLo
Copy link

MaCXyLo commented May 4, 2017

Hi,
the social links doesn't work if url parameter in the _config file is not set.

url                      : # the base hostname & protocol for your site e.g. "https://mmistakes.github.io"

if the parameter share: true is set, there must be a check that the url is set in the _config file.
otherwise the social buttons are broken.

idea: something similar like this error message while the compile process:

GitHub Metadata: No GitHub API authentication could be found. Some fields may be missing or have incorrect data.

@mmistakes
Copy link
Owner

Can't throw an error since that's part of Jekyll and the layouts are using Liquid. When hosting on GitHub Pages they automatically populate url for you regardless if you do or don't in _config.yml.

When you test locally it obviously will be blank. A bunch of stuff won't work if you don't assign a url. So not really sure if that's the theme's fault or just one of those Jekyll 101 things everyone should be aware of and do.

@mmistakes
Copy link
Owner

This issue has been marked as stale. Please feel free to reopen if:

  1. This is a bug and you can still reproduce using the latest version of the theme. Please reply with any additional information you have to keep the issue open.

  2. This is a feature request and can elaborate on why you feel more than 80% of users would find it beneficial.

Thanks for your contribution 😄.

koyumi0601 pushed a commit to koyumi0601/koyumi0601.github.io that referenced this issue Jul 31, 2023
* Optimize simple navigation cases

Fix inefficiency reported in feedback on v0.4.0.rc2 (see discussion mmistakes#958).

This PR:

* essentially reverts `_includes/nav.html` to v0.4.0.rc1
* preserves the ARIA labels added by mmistakes#950
* adds a test to optimize builds of sites that rely on `title` fields to order pages.

Building the `endoflife.date` site (130 pages) now takes only about 7 seconds.

Building the `machinetranslate.org` site ( 350 pages) takes about 7 minutes. (Without the added test, it takes just over 5 minutes: the condition of the test is merely to compare the size of two arrays, but that is apparently enough to prevent Jekyll from applying some optimization).

A warning is added to the docs about the need for numbers to be in quotes when used as title values.

* Update navigation-structure.md

A clarification is added to the docs about the need for numbers to be in quotes when used as title values.

* Simplify the control and data flow

- Defer concatenation of `string_order_pages` with `title_order_pages` until needed.
- Replace tests on size with tests for `empty`.
- Rename variables accordingly.

* Fix child nav order

This PR started from the navigation in RC1. Some cosmetic improvements had been made in RC2. This commit adds some of those changes to this PR.

It also fixes a bug (revealed by a new regression test) due to a reference to `node.child_nav_order` instead of `child.child_nav_order`, which prevented reversal of the order in children of children. Presumably a top-level reversal should apply only to direct children, and not to grandchildren. The latter interpretation would be very confusing in a deep multi-level hierarchy.

* Allow pages with numeric titles

An omitted `nav_order` value should default to the `title` value, regardless of its type. Jekyll 3 gives build errors when numbers and strings are sorted together. This commit drops the assumption that `title` values are always strings – a 404 page naturally has a numeric title. It updates the docs page accordingly.

The extra code does not affect the build time for the `endoflife.date` site (7 seconds). For the `machinetranslate` site, changing the title of the 404 page to a number increases the build time from 7 minutes to 9 minutes – the `nav_order` numbers on that site are program-generated in the range 1..1000, which might be atypical.

This commit has not yet been checked using the regression tests.

The gemspec used for testing specifies `spec.add_runtime_dependency "jekyll", "~> 3.8.5"`, and `Gemfile.lock` shows `jekyll (3.8.7)`.

* Update nav.html

Add comment about an optimization that will be possible in Jekyll 4.

* Update nav.html

- Update the comment about optimization possibility.
- TEMPORARILY add Jekyll 3 code for conditionally optimizing.

* Update nav.html

Minor improvements and cosmetic changes.

* Major revision

This update is based on extensive experimentation and profiling with alternative versions of the Liquid code used to build the main navigation panel.

Due to the fragility of Jekyll's optimizations, combining alternative approaches with conditionals turned out to be too expensive: merely adding a condition to check whether some array of pages is empty can add about 20% to the build time!

The current code avoids sorting pages on `nav_order` and `title` fields together. The standard way of doing that in Jekyll is to use the `group_by` filter; but extracting the sorted pages from the groups turned out to be too inefficient (as seen in RC1), as was generating links directly from the groups (in RC2).

Making all pages with `nav_order` values come before all those ordered by their `title` values is not ideal (it doesn't support tweaking the relative order of two pages in a list of pages ordered by their titles) but it appears to be necessary for efficient builds on large sites.

This version has not yet been fully tested for regression, but otherwise seems to give the expected navigation on the endoflife.date and machinetranslate websites. (I'm unable to install the Python-based how2data repository on my laptop, due to package version issues on Apple silicon).

Co-authored-by: Peter Mosses <18308236+pdmosses@users.noreply.github.com>
yesterz pushed a commit to yesterz/yesterz.github.io-test that referenced this issue Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants