Port to Boostrap 4 #246

varnerac · 2019-12-02T17:08:13Z

Do you have an appetite to move to the Bootstrap 4 version or hold it as a dev branch? We want to loosely track the upstream version.

There's a known issue this port. Highlighting of active tags does not work. I cannot find the CSS class to do this in the new Boostrapious Universal distribution.

varnerac · 2019-12-05T14:43:40Z

Just following up to see if you’d be interested in this PR or if it’s better for us to maintain the Bootstrap 4 version as a fork.

I know the eslint task failed. As best I can tell, the JavaScript files it complains about are basically the same ones already in the repo. Also, I tested this using the example site. Was anyone else able to test the branch on their machine?

salim-b · 2019-12-05T15:27:11Z

Just following up to see if you’d be interested in this PR or if it’s better for us to maintain the Bootstrap 4 version as a fork.

I'm not a maintainer of this theme but I strongly assume that your efforts to move it to bootstrap 4 are welcome... @ryanfox1985 ?

Was anyone else able to test the branch on their machine?

I'll have a look at it in the next few days!

salim-b · 2019-12-05T15:31:29Z

layouts/partials/widgets/tags.html

-            <li{{ if eq $current.RelPermalink ($name | urlize | lower | printf "/tags/%s/") }} class="active"{{ end }}>
-                <a href="{{ "tags/" | relURL }}{{ $name | urlize | lower }}"><i class="fas fa-tags"></i> {{ $name }}</a>
+            <li class="list-inline-item">
+                <a{{ if eq $current.RelPermalink ($name | urlize | lower | printf "/tags/%s/") }} class="active"{{ end }} href="{{ $.Site.BaseURL }}tags/{{ $name | urlize | lower }}"><i class="fas fa-tags"></i> {{ $name }}</a>


There's a known issue this port. Highlighting of active tags does not work.

I didn't test your PR and only briefly looked at the changes you made, but I think broken highlighting could be due to the fact that you don't assign the class active to the <li> element anymore (you chose to assign it to the <a> element instead). What's your reason to change this?

(same applies to the categories partial layouts/partials/widgets/categories.html)

I am not a frontend developer, so I am feeling my way through this. However, active category highlighting works. You'll see that once you test the example site.

This also won't be the first time I've written code that works and I am not sure why it works.

This is how it's handled on the upstream Boostrapious demo site here: https://demo.bootstrapious.com/universal/2-0-2/blog-medium.html

I've run your code in Hugo and it turns out you could just add the following CSS to the static/css/style.*.css files to enable proper tag highlighting (example for style.default.css):

.tag-cloud a.active { color: #ffffff; background-color: #4fbfa8; }

I don’t plan on making this change for this PR. I’ll look at what they do upstream.

salim-b · 2019-12-06T13:36:36Z

exampleSite/config.toml

@@ -127,11 +127,8 @@ paginate = 10
 # Enable or disable top bar with social icons


@ line 85 you should update the comment about the default theme color (now turquoise instead of light-blue)

salim-b · 2019-12-06T13:42:08Z

(...) if it’s better for us to maintain the Bootstrap 4 version as a fork

Basically, I think the changes are so extensive that it would be appropriate to have this bootstrap 4 port in a separate repository (at least for the time being). But I wonder what the maintainers here think, @ryanfox1985 @GeorgeWL @adrianmo ?

ryanfox1985 · 2019-12-11T17:40:07Z

layouts/partials/features.html

                                <i class="{{ .icon }}"></i>
                            </div>
                        {{ with $element.url }}
                        </a>
                        {{ end }}
-                        <h3>{{ $element.name | markdownify }}</h3>
+                        <h3 class="h4">{{ $element.name | markdownify }}</h3>


it sounds weird h3 and class h4.

Yeah. I just ripped this off from the Boostrapious live demo/raw distro. We download the raw distro from them and pay for a non-attribution license.

I leave the comment opened to remember this on the final check

ryanfox1985 · 2019-12-11T17:45:04Z

I'm not sure about this feature, how to manage it, what is the best.

Time ago I realized the authors from Universal theme have updated the theme and it's not easy to apply the new updates to the current theme.

Perhaps a new main branch or directly a new project.

ryanfox1985 · 2019-12-11T17:49:47Z

The new Universal version is already using Bootstrap v4 (v4.3.1)

varnerac · 2019-12-11T18:00:03Z

So, I’m happy to put this in a new repo. What I didn’t want to do is fork this without the consent of current maintainers. My preference is to fork it to hugo-universal-theme-2/hugo-universal-theme-2 rather than under our company. I’d also like to make some changes suggested in the current issue backlog, like shortcodes and managing images as resources rather than bare links. If there are no objections I’ll start the fork.

ryanfox1985 · 2019-12-12T13:16:30Z

I think the best is create a master_v2 in the same repository. And the release names use:

1.X.X to refer to version v1
2.X.X to refer to version v2

etc...

salim-b · 2019-12-12T13:56:11Z

I'd recommend to create a fresh repo as proposed by @varnerac (like hugo-universal-theme-2/hugo-universal-theme-2). Too much stale stuff around here (like open PRs from mid-2017...), therefore a fresh start seems appropriate to me. Just my 5 cents :)

ryanfox1985 · 2019-12-12T14:53:39Z

I'm not agree, if you take a look to other public repositories the different versions are in the same repository.

example jquery:

salim-b · 2019-12-12T16:18:11Z

I'm not agree (...)

No offence, but during the last two years there were periods during which almost no maintaining was done here. Which causes people to regularly place comments like this one.

But if you give appropriate write access to @varnerac , putting the bootstap 4 port into a master_v2 branch inside this repo seems fine to me, too. 😉

ryanfox1985 · 2019-12-13T09:28:49Z

@salim-b ,
sadly you are right, during the last two years this repository was no longer maintained but things can change.

Last months this repository was very active:

https://github.com/devcows/hugo-universal-theme/releases
I send a invitation to two new maintainers.

So let's see if we can maintain the repository in a good direction.

salim-b · 2019-12-13T10:09:31Z

I send a invitation to two new maintainers.

Ok, thank you! I can't promise anything but as long as I'll be using this theme myself I'll try to help here :)

ryanfox1985 · 2019-12-13T10:31:01Z

I changed the based branch to master_v2

ryanfox1985 · 2019-12-13T10:32:13Z

exampleSite/config.toml

@@ -143,35 +140,35 @@ paginate = 10
    enable = true
    # All carousel items are defined in their own files. You can find example items
    # at 'exampleSite/data/carousel'.
-    # For more informtion take a look at the README.
+    # For more information take a look at the README.


@varnerac I would appreciate misspelling words to be in another PR directly to master branch.

ryanfox1985 · 2019-12-13T10:34:03Z

layouts/partials/map.html

-        <input id="gmap-marker" value="{{ "img/marker.png" | relURL }}" />
+        <input type="hidden" id="gmap-lat" value="{{ .Site.Params.latitude }}" />
+        <input type="hidden" id="gmap-lng" value="{{ .Site.Params.longitude }}" />
+        <input type="hidden" id="gmap-marker" value="{{ .Site.BaseURL }}img/marker.png" />


is this change needed ? {{ "img/marker.png" | relURL }}

I'd strongly suggest to stick with {{ "img/marker.png" | relURL }} instead of {{ .Site.BaseURL }}img/marker.png. The latter results in an absolute URL which can cause portability issues (which one can work around using Hugo's --baseURL option; but if we can avoid the hassle from the beginning... 😉 )

ryanfox1985 · 2019-12-13T10:36:02Z

layouts/partials/scripts.html

-<script src="{{ "js/hpneo.gmaps.js" | relURL }}"></script>
-<script src="{{ "js/gmaps.init.js" | relURL }}"></script>
-<script src="{{ "js/front.js" | relURL }}"></script>
+<script src="{{ .Site.BaseURL }}js/hpneo.gmaps.js"></script>


same here {{ "js/gmaps.init.js" | relURL }} is this needed?

Yeah, same here, pls use relative URLs!

varnerac · 2019-12-17T22:23:46Z

Let me know if this works. I do not intend to change the CSS for .tag-cloud a.active at the moment. I will go back to Ondrej and see if he wants to fix it upstream. If so, I'll imitate how he fixes it upstream.

GeorgeWL · 2019-12-19T09:30:42Z

Error: add site dependencies: load resources: loading templates: "/Users/travis/build/devcows/hugo-universal-theme/layouts/partials/footer.html:65:1": parse failed: template: partials/footer.html:65: unexpected {{end}}

on the latest Travis Build of this.

varnerac · 2019-12-19T22:36:32Z

@GeorgeWL I just fixed that in my branch here

I am not sure why this PR hasn't picked it up and re-run Travis on it. My merge commits, which I lazily did on GitHub UI, botched the footers.

varnerac · 2019-12-20T17:07:34Z

@GeorgeWL Okay, if you click on "Details" on the TravisCI check, you'll see my latest commit passes. Unsure why it's not showing in the GitHub UI.

varnerac · 2019-12-20T18:44:46Z

Please don't merge this yet. While porting our site to the Bootstrap 4 version, we found some errors in the carousel.

simonsan · 2020-02-13T02:03:45Z

Any news about this?

varnerac · 2020-02-19T16:44:06Z

So, it depends on the definition of done. This works for the example Hugo project. However, it needs improvements for features not tested in the example app. Is the definition of done working with the example app, or works with everything?

rvoortman · 2023-01-11T08:40:26Z

This has been stale for some time now; is this going to be picked up again by someone? I'm not too familiar with the bootstrap upgrade, but I do know that version 5 is out and version 3.3.7 has has some CVE's at the moment that are not going to be fixed: https://security.snyk.io/package/npm/bootstrap. It might be useful to get this PR ongoing

Port to Boostrap 4

56f9349

salim-b reviewed Dec 5, 2019

View reviewed changes

salim-b reviewed Dec 6, 2019

View reviewed changes

ryanfox1985 reviewed Dec 11, 2019

View reviewed changes

ryanfox1985 changed the base branch from master to master_v2 December 13, 2019 10:30

ryanfox1985 reviewed Dec 13, 2019

View reviewed changes

varnerac and others added 2 commits December 17, 2019 17:14

Address reviewer comments.

32611a0

Merge branch 'master_v2' into migrate_to_bootstrap_4

7742dce

Fix error caused by my merge commit

52e5b76

salim-b mentioned this pull request Feb 25, 2020

Support for menus with sections and optional image #257

Closed

		@@ -127,11 +127,8 @@ paginate = 10
		# Enable or disable top bar with social icons

Port to Boostrap 4 #246

Are you sure you want to change the base?

Port to Boostrap 4 #246

Conversation

varnerac commented Dec 2, 2019 • edited

varnerac commented Dec 5, 2019

salim-b commented Dec 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salim-b commented Dec 6, 2019

ryanfox1985 Dec 11, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryanfox1985 Dec 13, 2019 • edited

Choose a reason for hiding this comment

ryanfox1985 commented Dec 11, 2019 • edited

ryanfox1985 commented Dec 11, 2019

varnerac commented Dec 11, 2019 • edited

ryanfox1985 commented Dec 12, 2019 • edited

salim-b commented Dec 12, 2019

ryanfox1985 commented Dec 12, 2019

salim-b commented Dec 12, 2019

ryanfox1985 commented Dec 13, 2019

salim-b commented Dec 13, 2019 • edited

ryanfox1985 commented Dec 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

varnerac commented Dec 17, 2019

GeorgeWL commented Dec 19, 2019

varnerac commented Dec 19, 2019

varnerac commented Dec 20, 2019

varnerac commented Dec 20, 2019

simonsan commented Feb 13, 2020

varnerac commented Feb 19, 2020

rvoortman commented Jan 11, 2023

varnerac commented Dec 2, 2019 •

edited

ryanfox1985 Dec 11, 2019 •

edited

ryanfox1985 Dec 13, 2019 •

edited

ryanfox1985 commented Dec 11, 2019 •

edited

varnerac commented Dec 11, 2019 •

edited

ryanfox1985 commented Dec 12, 2019 •

edited

salim-b commented Dec 13, 2019 •

edited