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

Added default theme #960

Merged
merged 9 commits into from
Sep 6, 2024

Conversation

joyao
Copy link
Member

@joyao joyao commented Aug 24, 2024

[Updated]

Issue

Add a new theme feature and fully resolved issues #931 and #934.

Description

As a developer, I want to be able to customize the theme of my website so that users can have a better browsing experience.

Changes:

  1. Gets the default theme values ​​from the site configuration file and leaves the user with the option to change them.
  2. If no value is set, the value 'system' is used.

Test Evidence

Description Expected Result Status
comment out darkMode and theme settings light light Pass
comment out theme settings &
disable darkMode
light light Pass
comment out theme settings &
enable darkMode
system system Pass
comment out darkMode settings &
enable theme without service settings
system system Pass
comment out darkMode settings &
disable theme settings
light light Pass
comment out darkMode settings &
enable theme service default settings
system system Pass
comment out darkMode settings &
enable theme settings & set light = false
dark dark Pass
comment out darkMode settings &
enable theme settings & set dark = false
light light Pass
comment out darkMode settings &
enable theme settings & set default = light
light light Pass
comment out darkMode settings &
enable theme settings & set default = dark
dark dark Pass
comment out darkMode settings &
enable theme settings
set light = false & set default = light
dark dark Pass
comment out darkMode settings &
enable theme settings
set dark = false & set default = dark
light light Pass
enable darkMode settings &
enable theme settings
follow theme settings follow theme settings Pass

Copy link

netlify bot commented Aug 24, 2024

Deploy Preview for toha-ci ready!

Name Link
🔨 Latest commit 7d6e045
🔍 Latest deploy log https://app.netlify.com/sites/toha-ci/deploys/66dad4772510660008806c61
😎 Deploy Preview https://deploy-preview-960--toha-ci.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 configuration.

@hossainemruz
Copy link
Member

Hi @joyao! Thank you for the PR. It looks good. I was just wondering if we should combine the theme options under single configuration instead of splitting into defaultTheme and darkMode?

I was thinking something like this:

params:
  features:
    theme:
      light: true # enable light theme. default "true"
      dark: true # enable dark theme. default "true"
      defaultTheme: light # can be either light, dark or system. default "system"

What do you think? This will require more work but I can update your PR. We also have to keep backward compatibility.

@joyao
Copy link
Member Author

joyao commented Aug 30, 2024

Hi @hossainemruz, that’s a good point and I agree with you. I can try to update if you're not in a hurry. Please let me know your thoughts.

@hossainemruz
Copy link
Member

Hi @hossainemruz, that’s a good point and I agree with you. I can try to update if you're not in a hurry. Please let me know your thoughts.

That would be great! Go ahead with the change. Just make sure we are not breaking existing sites.

@joyao
Copy link
Member Author

joyao commented Aug 31, 2024

Hi @hossainemruz, that’s a good point and I agree with you. I can try to update if you're not in a hurry. Please let me know your thoughts.

That would be great! Go ahead with the change. Just make sure we are not breaking existing sites.

For sure. This is the unchanging rule. :)

@joyao
Copy link
Member Author

joyao commented Sep 1, 2024

Hi @hossainemruz, features and comments have been updated based on what was discussed previously. Please check.

@hossainemruz
Copy link
Member

Hi @hossainemruz, features and comments have been updated based on what was discussed previously. Please check.

Thank you @joyao! I will take a look whenever get some time. Thank you for the amazing work.

Copy link
Member

@hossainemruz hossainemruz left a comment

Choose a reason for hiding this comment

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

Thank you @joyao! Looks good now.

@hossainemruz hossainemruz merged commit 7bc37c7 into hugo-toha:main Sep 6, 2024
11 of 12 checks passed
@joyao joyao deleted the feat/Add_default_theme_feature branch September 23, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants