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

Switch to Bootstrap v4.1.1 #30

Merged
merged 2 commits into from
Dec 3, 2018
Merged

Conversation

sivaraam
Copy link
Member

@sivaraam sivaraam commented Jun 24, 2018

This migrates the site to use a stabler and newer version of
Bootstrap (v4.1.1) from the deprecated v4-alpha. This has a few
side effects as noted below:

  • The navigation bar was tweaked a little to make it suit well with
    the newer version of the framework. While at it, the website title
    was moved to the left in smaller screens ensure it doesn't clash
    with the hamburger menu.

  • The external script tags for the newer version were moved to the
    end of the document to ensure they don't affect the page loading
    a lot. (The script is required only for the hamburger menu, shown
    in mobile devices)

  • Somme inline styles were moved to the stylesheet for the sake of
    consistency.

Fixes: #29

This migrates the site to use a stabler and newer version of
Bootstrap (v4.1.1) from the deprecated v4-alpha. This has a few
side effects as noted below:

- The navigation bar was tweaked a little to make it suit well with
  the newer version of the framework. While at it, the website title
  was moved to the left in smaller screens ensure it doesn't clash
  with the hamburger menu.

- The external script tags for the newer version were moved to the
  end of the document to ensure they don't affect the page loading
  a lot. (The script is required only for the hamburger menu, shown
  in mobile devices)

- Somme inline styles were moved to the stylesheet for the sake of
  consistency.
@sivaraam
Copy link
Member Author

Could be tested live at https://sivaraam.github.io/commons-app.github.io/

Copy link
Member

@tobias47n9e tobias47n9e left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

@sivaraam
Copy link
Member Author

Just FYI, I've used https://sivaraam.github.io/commons-app.github.io/ to host the changes of #31 for now.

@sivaraam
Copy link
Member Author

Ok, I just noticed a mild inconsistency in this change. The navbar isn't fixed anymore. Is that ok, or should I bring back the stickness?

@sivaraam
Copy link
Member Author

sivaraam commented Dec 2, 2018

Hi, It's been a long time since the change has been approved. Why hasn't it been approved yet?

@misaochan
Copy link
Member

Sorry about the delay. I am really not familiar with web dev, but since @tobias47n9e approved it, I will go ahead and merge. (He is the maintainer for the website AFAIK)

@misaochan misaochan merged commit c25e38c into commons-app:master Dec 3, 2018
@sivaraam sivaraam mentioned this pull request Feb 3, 2019
7 tasks
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.

None yet

4 participants