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

FIXED Issue 69: Theme-box is now on the Home page header. #71

Merged
merged 2 commits into from Nov 22, 2022

Conversation

a-parris21
Copy link

What does this PR do?

The theme-box has been moved to the header and no longer overlap the tutorial boxes, but the theme-box is still unique to the Home page.

Related Issue

Resolves the primary problem described in Issue #69.

Screenshots of changes made

Home Page before changes

image

Home Page after changes

image

Description of code changes

Initially, I moved the theme-box's HTML from the Home page's index.html file to the include\header.html file, as a list item within the <div class="menu"> element.
This resulted in the theme-box appearing on the header on each page. However, the theme-box lost the ability to actually change the website's theme. The functionality within app.js is not applied to the theme-box if the theme-box is included from a separate HTML file via the include-html attribute on an element (e.g. <div include-html="include\header.html"></div>).

So, I moved the HTML code from the include\header.html file into the Home page's index.html file and discarded the changes I made to the header.html file (as such, the header.html file does not have any changes committed to it).
I tested this version and the theme-box appears on the header, thus it does not overlap tutorial boxes anymore (as described in issue #69) and it works correctly (being able to change the theme as intended).

Note that: with this method, the theme-box only appears on the Home page and is not available on the individual tutorial pages.

@netlify
Copy link

netlify bot commented Nov 22, 2022

Deploy Preview for html-github ready!

Name Link
🔨 Latest commit ecff5eb
🔍 Latest deploy log https://app.netlify.com/sites/html-github/deploys/637c4796d4c93f00088e5601
😎 Deploy Preview https://deploy-preview-71--html-github.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Message that will be displayed on users' first pull request

@Diptenusarkar Diptenusarkar linked an issue Nov 22, 2022 that may be closed by this pull request
@Diptenusarkar
Copy link
Owner

I found this problem on it.
Screenshot_2022-11-22-07-53-53-364_com.duckduckgo.mobile.android.jpg

@a-parris21
Copy link
Author

a-parris21 commented Nov 22, 2022

I just pushed a change to the issue-69 branch.
Can you check if the problem (icons listed horizontally on mobile instead of vertically) has been resolved?

Copy link
Owner

@Diptenusarkar Diptenusarkar left a comment

Choose a reason for hiding this comment

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

Good Job 👍

@Diptenusarkar Diptenusarkar merged commit b6d9a00 into Diptenusarkar:main Nov 22, 2022
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.

UI/UX Edit: theme box overlaps the tutorial boxes
2 participants