Skip to content

fix: make logo optional#130

Merged
prashant-gurung899 merged 3 commits into
mainfrom
make-logo-optional
May 15, 2026
Merged

fix: make logo optional#130
prashant-gurung899 merged 3 commits into
mainfrom
make-logo-optional

Conversation

@prashant-gurung899
Copy link
Copy Markdown
Contributor

@prashant-gurung899 prashant-gurung899 commented Mar 30, 2026

Description

This PR makes the logo to be optional.
And also added an unit test to render the template without the logo.

Related Issue

Motivation and Context

How Has This Been Tested?

  • Locally

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Tests only (no source changes)
  • Documentation only (no source changes)
  • Maintenance (e.g. dependency updates or tooling)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation updated

Copy link
Copy Markdown
Member

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

Let's also add some tests

Comment thread src/App.vue Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to make the presentation logo optional so that slides don’t display the “Logo” alt text when no logo is provided in front matter metadata.

Changes:

  • Added a setLogo hook intended to hide the logo image when no logo is defined.
  • Updated template-application flow to invoke logo handling when templates are enabled.
  • Updated .gitignore to ignore the generated package.json.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/App.vue Adds logo-hiding logic when templates are applied.
.gitignore Ignores generated package.json.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/App.vue Outdated
Comment thread src/App.vue
@prashant-gurung899 prashant-gurung899 force-pushed the make-logo-optional branch 5 times, most recently from 1907fbf to f865e66 Compare May 13, 2026 10:31
Comment thread src/App.vue Outdated
Comment thread src/App.vue Outdated
el.style.color = color
})
}
function setLogo(frontMatter) {
Copy link
Copy Markdown
Member

@saw-jan saw-jan May 14, 2026

Choose a reason for hiding this comment

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

hmm, the reason for showing the logo alt text is how we have the template.
IMO, we should fix the default template instead of removing logo container from html here.
Let the template files handle the rendering.

logo container should be enclosed by {{#metadata.lgo}}...{{/metadata.logo}}

<div class="logo">
<img src="{{{ metadata.logo }}}" alt="Logo">
</div>
</div>

Similiar to about-us
{{#metadata.aboutUs}}
<div class="info-box">
<h3>{{ title }}</h3>
<p>{{ text }}</p>
</div>
<div class="divider"></div>
{{/metadata.aboutUs}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If that's the case then i have removed the function and updated the templates 💯

Signed-off-by: prashant-gurung899 <prasantgrg777@gmail.com>
Signed-off-by: prashant-gurung899 <prasantgrg777@gmail.com>
Comment thread public/templates/about-us-template.html Outdated
Comment thread public/templates/about-us-template.html Outdated
@prashant-gurung899 prashant-gurung899 force-pushed the make-logo-optional branch 2 times, most recently from 8903210 to a2d05be Compare May 15, 2026 06:13
Comment on lines +266 to +268
<div class="logo">
<img src="" alt="Logo">
</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to wrap the logo div in {{#metadata.logo}}...{{/metadata.logo}} in the templateHtmlMap in the test code. Then this should also be removed from the snapshot.

Signed-off-by: prashant-gurung899 <prasantgrg777@gmail.com>
@prashant-gurung899 prashant-gurung899 merged commit 1225296 into main May 15, 2026
2 checks passed
@prashant-gurung899 prashant-gurung899 deleted the make-logo-optional branch May 15, 2026 09:32
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.

[Bug]: Logo should be optional

5 participants