Skip to content

GLASGOW | Jan-26-ITP | Prakash Dcosta | Sprint 3 | Quote Generator #1107

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.

Sign up for GitHub

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

Open
dcostaprakash wants to merge 3 commits into
base: main
Choose a base branch
from
+109 −5
Open

GLASGOW | Jan-26-ITP | Prakash Dcosta | Sprint 3 | Quote Generator #1107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f8f206
GLASGOW | Jan-26-ITP | Prakash Dcosta | Sprint 3 | Quote Generator
dcostaprakash Mar 26, 2026
0e554ad
I have made the modifications as per the review.
dcostaprakash Apr 4, 2026
3619a77
Made the changes as per the review
dcostaprakash Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions Sprint-3/quote-generator/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,23 @@
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Title here</title>
<title>Quote generator app</title>
<link rel="stylesheet" href="style.css" />
<script defer src="quotes.js"></script>
</head>
<body>
<h1>hello there</h1>
<p id="quote"></p>
<p id="author"></p>
<button type="button" id="new-quote">New quote</button>
<div class="page-wrapper">
<h1>Your Quote for the day</h1>
<!-- Quote container added -->
<div class="quote-container">

<!-- Quote text section -->
<div class="quote-text">
<p id="quote"></p>
<p id="author"></p>
</div>
<button type="button" id="new-quote">New quote</button>
</div>
Comment on lines +12 to +22
Copy link
Copy Markdown

@Luro91 Luro91 Apr 4, 2026

Choose a reason for hiding this comment

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

How could you make it more clear what HTML elements are nested inside the new divs?

Copy link
Copy Markdown
Author

@dcostaprakash dcostaprakash Apr 4, 2026

Choose a reason for hiding this comment

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

Got it. I can add comments and i can make it visually better. I have modified it. Thank you

Copy link
Copy Markdown

@Luro91 Luro91 Apr 5, 2026

Choose a reason for hiding this comment

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

I was referring to code indentation. With the correct indentation the comments are not needed. indentation is a standard that makes it easy to see visually which elements are on which hierarchy. Tools like the IDE also allow code folding based on indentation

Copy link
Copy Markdown
Author

@dcostaprakash dcostaprakash Apr 5, 2026

Choose a reason for hiding this comment

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

I have made the change now. Pl check. I will also go go through IDE thoroughly. Thank you

Copy link
Copy Markdown

@Luro91 Luro91 Apr 6, 2026

Choose a reason for hiding this comment

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

The indentation looks good now. If you run prettier it should always do the correct formatting for you and you do not have to think about it

</div>
</body>
</html>
15 changes: 15 additions & 0 deletions Sprint-3/quote-generator/quotes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,4 +490,19 @@ const quotes = [
},
];

function displayRandomQuote() {
const randomQuote = pickFromArray(quotes);

document.getElementById("quote").textContent = randomQuote.quote;
document.getElementById("author").textContent = randomQuote.author;
}

// A new quote is shown on page load
displayRandomQuote();

//Button click
document
.getElementById("new-quote")
.addEventListener("click", displayRandomQuote);

// call pickFromArray with the quotes array to check you get a random quote
79 changes: 79 additions & 0 deletions Sprint-3/quote-generator/style.css
View file
Open in desktop
Copy link
Copy Markdown

@Luro91 Luro91 Apr 4, 2026

Choose a reason for hiding this comment

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

An advanced topic regarding UX:
At the moment the boxes containing quotes have various sizes depending on the text. This also leads to the button changing the position. (it would be nice if the button would stay in position and the user would not have to move the mouse to click it again).
How could you accomplish this?

Image Image

Copy link
Copy Markdown
Author

@dcostaprakash dcostaprakash Apr 4, 2026

Choose a reason for hiding this comment

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

Understood. I made the following changes to incorporate it. I increased the quote-container height and max-width, made the display to flex and further the quote-text i made it to overflow with minimum heing 0 so that it can scroll and not disturb the button. Thanks

Copy link
Copy Markdown

@Luro91 Luro91 Apr 5, 2026

Choose a reason for hiding this comment

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

The box size still changes in width and make the button move in horizontal direction.
This is an optional improvement that you could do. In this case the list of quotes is defined so you can design the box for the longest quote and it will fit all of them.

Screenshot from 2026-04-05 07-18-33 Screenshot from 2026-04-05 07-18-26

Copy link
Copy Markdown
Author

@dcostaprakash dcostaprakash Apr 5, 2026

Choose a reason for hiding this comment

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

I made the modifications and tested it. Pl see if its alright now. Thanks

Copy link
Copy Markdown

@Luro91 Luro91 Apr 6, 2026

Choose a reason for hiding this comment

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

Looks good now

Original file line number Diff line number Diff line change
@@ -1 +1,80 @@
/** Write your CSS in here **/
body {
margin: 0;
font-family: Georgia, "Times New Roman", serif;
background-color: #f59e0b;
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
}
/* h1 changes */
h1 {
color: #050301;
text-align: center;
margin-bottom: 40px;
font-weight: normal;
}

/* Container card */
.quote-container {
background-color: #d9d9d9;
width: 1200px;
max-width: none;
padding: 60px;
box-sizing: border-box;
text-align: left;
display: flex;
flex-direction: column;
justify-content: space-between;
height: 500px;

}

/* Quote text */
#quote {
font-size: 32px;
color: #f59e0b;
line-height: 1.4;
margin-bottom: 30px;
position: relative;
}

/* Big quote mark */
#quote::before {
content: "“";
font-size: 80px;
position: absolute;
left: -40px;
top: -20px;
color: #f59e0b;
}

/* Author */
#author {
text-align: right;
font-size: 20px;
color: #130d04;
margin-bottom: 30px;
}

/* Button */
#new-quote {
background-color: #f59e0b;
color: white;
border: none;
padding: 15px 25px;
font-size: 16px;
cursor: pointer;
align-self: flex-end;
}

#new-quote:hover {
background-color: #d97706;
}
/* Making the new area scrollable*/
.quote-text {
flex-grow: 1;
overflow-y: auto;
min-height: 0;
}
Loading