Skip to content

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

Open
dcostaprakash wants to merge 2 commits intoCodeYourFuture:mainfrom
dcostaprakash:Quote-Generator
Open

GLASGOW | Jan-26-ITP | Prakash Dcosta | Sprint 3 | Quote Generator#1107
dcostaprakash wants to merge 2 commits intoCodeYourFuture:mainfrom
dcostaprakash:Quote-Generator

Conversation

@dcostaprakash
Copy link
Copy Markdown

@dcostaprakash dcostaprakash commented Mar 26, 2026

Changelist
I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide
my coursework

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@dcostaprakash dcostaprakash added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 26, 2026
@Luro91 Luro91 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 4, 2026
Comment on lines +12 to +17
<h1>Your Quote for the day</h1>
<div class="quote-container">
<p id="quote"></p>
<p id="author"></p>
<button type="button" id="new-quote">New quote</button>
</div>
Copy link
Copy Markdown

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

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

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

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

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

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

@Luro91 Luro91 added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Apr 4, 2026
@dcostaprakash dcostaprakash added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Apr 4, 2026
@Luro91 Luro91 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Apr 5, 2026
@Luro91
Copy link
Copy Markdown

Luro91 commented Apr 5, 2026

I only now noticed your commit messages. When I read the commit messages I don't know what the change was.
Commit messages should give everyone an understanding on what was changed in the commit without having to look at the actual file changes. It's also good practice to separate changes into separate commits. (For example one for adjusting the box size and one for changing the html structure)
You can also give context in the message about the reason for the change.

I recommend reading this article on commit messages: https://chris.beams.io/git-commit

@Luro91 Luro91 added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Apr 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants