You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/02/24 06:57:10 UTC

[GitHub] [apisix-website] SkyeYoung opened a new pull request #902: fix: switch color modes in multiple tabs

SkyeYoung opened a new pull request #902:
URL: https://github.com/apache/apisix-website/pull/902


   Fixes: #871
   
   Changes:
   
   Add `isDarkTheme` to `useEffect`'s deps array.
   
   Screenshots of the change:
   
   none


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] juzhiyuan commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814400255



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       How about using `setLightTheme(!isDarkTheme)`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] netlify[bot] commented on pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
netlify[bot] commented on pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#issuecomment-1049549992


   ๐Ÿ‘ท Deploy Preview for *apache-apisix* processing.
   
   
   ๐Ÿ”จ Explore the source changes: 2e8cd81077acb3e4b3c7afb489822c231b6a6b90
   
   ๐Ÿ” Inspect the deploy log: [https://app.netlify.com/sites/apache-apisix/deploys/62172c389ec20a00070e1ae2](https://app.netlify.com/sites/apache-apisix/deploys/62172c389ec20a00070e1ae2)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814417288



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       @juzhiyuan so I don't think it should be changed. ๐Ÿ˜… 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] juzhiyuan merged pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #902:
URL: https://github.com/apache/apisix-website/pull/902


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#issuecomment-1049554496


   @bzp2010 please give me some suggestions and reviews ๐Ÿ˜ณ
   
   Anyone is welcome to review this pr. Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] netlify[bot] edited a comment on pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
netlify[bot] edited a comment on pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#issuecomment-1049549992


   โœ”๏ธ Deploy Preview for *apache-apisix* ready!
   
   
   ๐Ÿ”จ Explore the source changes: 2e8cd81077acb3e4b3c7afb489822c231b6a6b90
   
   ๐Ÿ” Inspect the deploy log: [https://app.netlify.com/sites/apache-apisix/deploys/62172c389ec20a00070e1ae2](https://app.netlify.com/sites/apache-apisix/deploys/62172c389ec20a00070e1ae2)
   
   ๐Ÿ˜Ž Browse the preview: [https://deploy-preview-902--apache-apisix.netlify.app](https://deploy-preview-902--apache-apisix.netlify.app)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814415366



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       @juzhiyuan Sorry, I made a mistake. The intent of this part of the code is to reset the color mode to `light mode`. The above changes will not achieve this goal
   
   If you mean, change `setLightTheme(true)`  to `setLightTheme(!isDarkTheme)`. I think it might be clearer to use `ture` as a parameter.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814410086



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       Do you mean๏ผš
   ```diff
   - if(isDarkTheme){
   -   setLightTheme(true);
   - }
   + setLightTheme(!isDarkTheme)
   ```
   I have no plans to change this part of the code. ๐Ÿ˜ณ
   But, I'll try it.
   I think this might make the function execute more times. I'll check it right way.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814410086



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       @juzhiyuan Do you mean๏ผš
   ```diff
   - if(isDarkTheme){
   -   setLightTheme(true);
   - }
   + setLightTheme(!isDarkTheme)
   ```
   I have no plans to change this part of the code. ๐Ÿ˜ณ
   But, I'll try it.
   I think this might make the function execute more times. I'll check it right way.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814410086



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       @juzhiyuan Do you mean๏ผš
   ```diff
   - if(isDarkTheme){
   -   setLightTheme(true);
   - }
   + if(isDarkTheme){
   + setLightTheme(!isDarkTheme)
   + }
   ```
   I have no plans to change this part of the code. ๐Ÿ˜ณ
   But, I'll try it.
   I think this might make the function execute more times. I'll check it right way.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix-website] SkyeYoung commented on a change in pull request #902: fix: switch color modes in multiple tabs

Posted by GitBox <gi...@apache.org>.
SkyeYoung commented on a change in pull request #902:
URL: https://github.com/apache/apisix-website/pull/902#discussion_r814415366



##########
File path: website/src/pages/index.js
##########
@@ -28,14 +28,14 @@ const useWindowSize = () => {
 const ThemeResetComponent = () => {
   const {isDarkTheme, setLightTheme, setDarkTheme} = useThemeContext();
 
-  useEffect(() => {    
+  useEffect(() => {
     const children = document.querySelector(".navbar__items--right").childElementCount;
     document.querySelector(".navbar__items--right").childNodes[children-2].style.display = "none";
 
     if(isDarkTheme) {

Review comment:
       @juzhiyuan Sorry, I made a mistake. The intent of this part of the code is to reset the color mode to `light mode`. The above changes will not achieve this goal
   
   If you mean, just change `setLightTheme(true)`  to `setLightTheme(!isDarkTheme)`. I think it might be clearer to use `ture` as a parameter.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org