Skip to content

London | 26-ITP-May | Raihan Sharif | Sprint 3 | Programmer Humour#494

Open
RaihanSharif wants to merge 3 commits into
CodeYourFuture:mainfrom
RaihanSharif:feature/xkcd
Open

London | 26-ITP-May | Raihan Sharif | Sprint 3 | Programmer Humour#494
RaihanSharif wants to merge 3 commits into
CodeYourFuture:mainfrom
RaihanSharif:feature/xkcd

Conversation

@RaihanSharif
Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • 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

Changelist

Completed. Able to show comic, or error message if fetch fails. Shows fetch JSON in console.

@RaihanSharif RaihanSharif added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Data-Flows The name of the module. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels May 8, 2026
Copy link
Copy Markdown
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

LGTM - you may want to create the <img> and <p> tags when you get a result, rather than having an empty one and setting it - having empty placeholder elements is a bit messy because if you read the page you may notice them (e.g. the browser takes a little time to decide to render them, a screen-reader may say they're there, etc)

@illicitonion illicitonion added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels May 14, 2026
@RaihanSharif
Copy link
Copy Markdown
Author

Thank you for that feedback. I was actually thinking something similar but I think read a long time ago that doing that might be inefficient.

Probably is under some very specific scenarios that are not relevant to this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Data-Flows The name of the module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants