Skip to content
This repository was archived by the owner on Dec 15, 2020. It is now read-only.

added Bootstrap, jQuery and Random Quote #17

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

Conversation

arsho
Copy link

@arsho arsho commented Mar 26, 2017

See LIVE here: http://arsho.github.io/up1pr/
I have added the following things:

  • Bootstrap CDN (version 3.3.7)
  • jQuery CDN (version 3.2.1)
  • Random quote

@woubuc
Copy link
Contributor

woubuc commented Mar 27, 2017

Why make the visitor load the entirety of Bootstrap and jQuery for this? I think a "random quote" box can just as easily be done with a few lines of css and vanilla js.

@dakebl
Copy link
Contributor

dakebl commented Mar 29, 2017

@arsho As much as I enjoy using Bootstrap, maybe we should come up with our own CSS for this project. We could work on building our own Bootstrap-esq components library though to keep it DRY?.

jQuery would probably be a good additions though, as it adds a lot of fixes for things we would otherwise have to reinvent ourselves.

Perhaps you could add a separate random quote page to the project?

@arsho
Copy link
Author

arsho commented Mar 30, 2017

So, should I remove Bootstrap, keep jQuery and create a separate random quote page? I thought it would be nice to see a random quote now and then in welcome page.

@dakebl
Copy link
Contributor

dakebl commented Mar 30, 2017

@arsho Add the quotes wherever you like 😄
I only suggest another page to leverage the available real estate. I would suggest going jQuery only though, as above.

@CLAassistant
Copy link

CLAassistant commented Jul 2, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants