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

[Website] Update deps #449

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

[Website] Update deps #449

wants to merge 1 commit into from

Conversation

bkmgit
Copy link
Contributor

@bkmgit bkmgit commented Jan 5, 2024

No description provided.

"jquery": "^3.5.1",
"popper.js": "^1.16.1",
"webpack": "^5.76.0",
"bootstrap": "^5.3.2",
Copy link
Member

Choose a reason for hiding this comment

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

Updating Bootstrap 4 -> 5 may break design. Could you check it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems ok. Most of the documentation is built in the other Arrow repositories.

Copy link
Member

Choose a reason for hiding this comment

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

Could you share what you checked?
This PR also updates jekyll-feed. So we need to check at least https://arrow.apache.org/feed.xml too.

Copy link
Member

Choose a reason for hiding this comment

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

Providing preview is also helpful.
See also: https://github.com/apache/arrow-site#forks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current preview workflow at https://github.com/apache/arrow-site/blob/main/.github/workflows/deploy.yml fails with newer Jekyll as the one used by GitHub is older.

The current site can be previewed at https://bkmgit.github.io/arrow-site - without changes

Associated workflow is at:
https://github.com/bkmgit/arrow-site/blob/bkmgit-patch-1/.github/workflows/preview.yml
Can make a pull request to add this so that creating preview sites is easier.

Copy link
Member

Choose a reason for hiding this comment

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

The former isn't problem because we have 2 posts on 2023-11-09.

The latter will be fixed by the following. Could you send a separated PR?

diff --git a/_posts/2023-08-05-datafusion_fast_grouping.md b/_posts/2023-08-05-datafusion_fast_grouping.md
index f8b5a50e5ef..57fc1b70d87 100644
--- a/_posts/2023-08-05-datafusion_fast_grouping.md
+++ b/_posts/2023-08-05-datafusion_fast_grouping.md
@@ -25,7 +25,7 @@ limitations under the License.
 {% endcomment %}
 -->
 
-<!--- Converted from Google Docs using https://www.buymeacoffee.com/docstomarkdown --->
+<!-- Converted from Google Docs using https://www.buymeacoffee.com/docstomarkdown -->
 
 ## Aggregating Millions of Groups Fast in Apache Arrow DataFusion
 

Copy link
Member

Choose a reason for hiding this comment

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

The preview site has <meta name="generator" content="Jekyll v4.2.0" />.
Is it really generated with Jekyll 4.3.3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR to fix rss #461

Thanks, had served pages from incorrect directory. Needed to add base_url to config to
get correct urls in the preview. Using environment variables seems to repeat the base_url
so have put this in the extra_config.yml file

Preview at https://bkmgit.github.io/arrow-site/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to check navbar, other things seem to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kou Dropdowns work now. Let me know if anything else is needed.

@bkmgit bkmgit marked this pull request as ready for review January 5, 2024 17:50
Gemfile Outdated Show resolved Hide resolved
kou pushed a commit that referenced this pull request Jan 6, 2024
Use the latest Jekyll so the docs build with the latest Ruby version.

We'll update other dependencies by #449 .
@bkmgit bkmgit requested a review from kou January 8, 2024 08:08
@bkmgit bkmgit force-pushed the Update-deps branch 11 times, most recently from 305e341 to e04f961 Compare January 9, 2024 07:21
@jorisvandenbossche
Copy link
Member

A few things I noticed on the preview:

1. The labels on blog posts have a wrong color that make them invisible (https://bkmgit.github.io/arrow-site/blog/2023/12/18/14.0.2-release/):

image

versus (https://arrow.apache.org/blog/2023/12/18/14.0.2-release/):

image

2. The last dropdown (ASF links) is too close to the border of the window such that it slightly falls off:

image

versus:

image

It seems that the navbar no longer has left/right padding in the updated bootstrap (also the logo on the left side is closer to the side)

3. On the landing page, the top bar is now a slightly darker grey than the the background of the big logo banner, while before this was they same color (so you didn't visually distinguish the navbar on the landing page from the big banner). Is that change intentional?

4. In the mobile layout, the navigation doesn't work (clicking the "hamburger" menu doesn't do anything for me):
Screenshot from 2024-01-09 09-17-40

@bkmgit bkmgit force-pushed the Update-deps branch 9 times, most recently from b8310ec to 49f7863 Compare January 9, 2024 14:08
@bkmgit
Copy link
Contributor Author

bkmgit commented Jan 9, 2024

@jorisvandenbossche Thanks for the feedback.

  1. Updated tags on blog template, so they are now visible
  2. Updated the tag and added a margin, hopefully ok now
  3. The colors in bootstrap have changed to improve contrast. Changed the background of the big banner to match. Suggestions for other colors are welcome.
  4. Updated a tag that was not previously updated. Menu works on small screens.

Other suggestions are welcome. Site can be previewed at:
https://bkmgit.github.io/arrow-site/

@bkmgit
Copy link
Contributor Author

bkmgit commented Jan 16, 2024

@jorisvandenbossche In the upgraded version all hyperlinks are underlined, otherwise it seems quite similar. Can figure out how to remove the underlines in hyperlinks.

@bkmgit bkmgit force-pushed the Update-deps branch 2 times, most recently from a26d037 to 50ed23e Compare April 21, 2024 19:37
Put base url in Jekyll config to ensure rss feed has correct path
Popper needs to be explicitly imported
Bootstrap5 bg-dark color has changed, so need to match landing page
title background color
Bootstrap5 underlines links by default, use css not to underline links
unless hovered over
Bootstrap 4 -> 5 migration
Replace data-toggle by data-bs-toggle
Replace mr- by me-
Replace right- by end-
Put a container round navbar
Replace badge-secondary by bg-secondary
Replace ml-auto by ms-auto
Specify bootstrap javascript requirements
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.

4 participants