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

[New feature] Quote fetcher #8

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Glomels
Copy link
Contributor

@Glomels Glomels commented Oct 2, 2019

Display a quote at the top from a local array of quotes.
Click on the quote div to trigger a quote change.
Quote automatically changes every 15 seconds.

Limited to 150 character or less for responsive purposes.

@akshat157
Copy link
Owner

@Glomels This feature is good but on testing, it seems like the changeQuote function is going haywire after a few quote changes. It starts changing the quotes way too rapidly.

Also, the placement looks fine but fonts and alignment seems to be lacking a lot in the current state of the feature.

@Glomels
Copy link
Contributor Author

Glomels commented Oct 2, 2019

@Glomels This feature is good but on testing, it seems like the changeQuote function is going haywire after a few quote changes. It starts changing the quotes way too rapidly.

Also, the placement looks fine but fonts and alignment seems to be lacking a lot in the current state of the feature.

I'm using the same font as the buttons, seemed fine to me.
Should it be something more cursive or artsy?

@Glomels
Copy link
Contributor Author

Glomels commented Oct 2, 2019

I think it looks good this way. I decided to put a shorter character limit when on mobile view by getting the viewport width with javascript.
Generally fonts are bigger and have other small changes, get smaller under 767px screen size.

@akshat157
Copy link
Owner

Aesthetics were secondary. The major problem was with the quotes rapidly and randomly changing after a while. I'm actually viewing it on desktop currently, since I didn't plan a mobile version yet but yes, its good to see you already took it into account. No worries, I'll review it and make the required changes in a while.
Thanks for the contribution! 😄

@akshat157 akshat157 deleted the branch akshat157:main October 3, 2021 17:50
@akshat157 akshat157 closed this Oct 3, 2021
@akshat157 akshat157 reopened this Oct 3, 2021
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.

2 participants