add next/pre button to switch picture - #4
Conversation
|
@creazyboyone Thanks for creating this PR. Much appreciated. I noticed that there is some duplicated code in it. Could you simplify it first? |
|
ok I've solved it now |
kfstorm
left a comment
kfstorm
left a comment
There was a problem hiding this comment.
Sorry for the late review. Do you mind post a picture of the updated app? A video or GIF showing the interactivity would be better. I don't have a Windows dev machine right now. So it's not easy for me to build this PR and check for behavior changes.
| } | ||
| else | ||
| { | ||
| _task = DoWork(_cts.Token); |
There was a problem hiding this comment.
When refreshTime is 0, I think we should cancel the current work before starting a new work.
| } | ||
|
|
||
| public void Run() | ||
| public async void Run(int refrashTime) |
There was a problem hiding this comment.
typo
| public async void Run(int refrashTime) | |
| public async void Run(int refreshTime) |
| public async void Run(int refrashTime) | ||
| { | ||
| if (_task != null) | ||
| if (refrashTime > 0) |
There was a problem hiding this comment.
I don't think it's a good idea to use refreshTime == 0 to represent a force reload. The Run method here is to start a background thread which periodically check for updates and reload the wallpaper. For one time reload, we should cancel the current task and call DoWork directly in ReflashPicture method.
| prePic.Name = "PrePic"; | ||
| prePic.Size = new System.Drawing.Size(75, 32); | ||
| prePic.TabIndex = 7; | ||
| prePic.Text = "上一个"; |
There was a problem hiding this comment.
| prePic.Text = "上一个"; | |
| prePic.Text = "上一张"; |
| nextPic.Name = "NextPic"; | ||
| nextPic.Size = new System.Drawing.Size(75, 32); | ||
| nextPic.TabIndex = 8; | ||
| nextPic.Text = "下一个"; |
There was a problem hiding this comment.
| nextPic.Text = "下一个"; | |
| nextPic.Text = "下一张"; |
|
|
||
| private void preButton_Click(object sender, EventArgs e) | ||
| { | ||
| if(Constants.PictureIndex > 0) |
There was a problem hiding this comment.
We should disable the button if Constants.PictureIndex is already zero.
| { | ||
| if(Constants.PictureIndex > 0) | ||
| { | ||
| Program.ReflashPicture(--Constants.PictureIndex); |
There was a problem hiding this comment.
PictureIndex shouldn't be in Constants anymore because it's not a constant now.
2 participants
add two buttons which can switch next picture or the previous