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

Carousel: Changed Timer and fixed bug in MoveTo #1913

Merged
merged 31 commits into from Jun 18, 2021
Merged

Carousel: Changed Timer and fixed bug in MoveTo #1913

merged 31 commits into from Jun 18, 2021

Conversation

Yomodo
Copy link
Contributor

@Yomodo Yomodo commented Jun 16, 2021

This PR improves upon the MudCarousel component:

-Replaced System.Timers.Timer with System.Threading.Timer (excuse my exaggeration);
-Replaced most sync methods with async methods;
-Fixed a bug in the MoveTo() method of the Delimiters;

To reproduce: Carousel example: Click items: 2nd, 1st, 1st: the 2nd item is briefly visible.
(or 3rd, 2nd, 2nd: to see the 3rd item)

@Yomodo Yomodo changed the title Carousel timer Carousel Improvements Jun 16, 2021
* Added Table Virtualization.

* Fix formatting...

* Missing @using for virtualize

* Use MudVirtualize instead of Virtualize

* Fixed MudVirtualize API test

* Removed unused usings

Co-authored-by: Meinrad Recheis <meinrad.recheis@gmail.com>
@henon
Copy link
Collaborator

henon commented Jun 17, 2021

@HClausing would you like to review this?

@HClausing
Copy link
Member

Sure!

Yomodo and others added 10 commits June 17, 2021 15:49
* Added "Required" argument to Autocomplete

* Added MudAutocomplete and MudSelect unit tests for the Required attribute.

* Refined unit tests

* Added "Required" argument to Autocomplete

* Added MudAutocomplete and MudSelect unit tests for the Required attribute.

* Refined unit tests

Co-authored-by: Jeffrey Jangli <adminjja@KPNWP.onmicrosoft.com>
, #1835)

* Select and Autocomplete improvements

* -Select Adornment.End now variable;
-Select/Autocomplete enabled AdornmentIcon;

* merge

* -Switched MudSelect and MudAutocomplete CloseIcon <> OpenIcon values;
-

* Switched MudSelect and MudAutocomplete CurrentIcon condition

* -Docs refinment: Added AdornmentIcon, CloseIcon and OpenIcon examples for MudSelect and MudAutocomplete;
-Docs refinment: Set ShowCode="false" for Variants example;

* -Autcomplete: Added call to UpdateIcon() from IsOpen.set and removed other calls to UpdateIcon;
-Removed test app;

* MudSelect: Added @attributes="UserAttributes" to satisfy unit test.

* Made CurrentIcon private and renamed to _currentIcon

* Removed test app

* -Changed method MudAutocomplete.UpdateIcon() to private;
-Changed method MudSelect.UpdateIcon() to private;
-Changed property MudSelect.CurrentIcon to private and renamed it to _currentIcon;

* Select and Autocomplete improvements

* -Select Adornment.End now variable;
-Select/Autocomplete enabled AdornmentIcon;

* merge

* -Switched MudSelect and MudAutocomplete CloseIcon <> OpenIcon values;
-

* Switched MudSelect and MudAutocomplete CurrentIcon condition

* -Docs refinment: Added AdornmentIcon, CloseIcon and OpenIcon examples for MudSelect and MudAutocomplete;
-Docs refinment: Set ShowCode="false" for Variants example;

* -Autcomplete: Added call to UpdateIcon() from IsOpen.set and removed other calls to UpdateIcon;
-Removed test app;

* MudSelect: Added @attributes="UserAttributes" to satisfy unit test.

* Made CurrentIcon private and renamed to _currentIcon

* Removed test app

* -Changed method MudAutocomplete.UpdateIcon() to private;
-Changed method MudSelect.UpdateIcon() to private;
-Changed property MudSelect.CurrentIcon to private and renamed it to _currentIcon;

Co-authored-by: Jeffrey Jangli <adminjja@KPNWP.onmicrosoft.com>
* Add MarkupString for Tab text

* Added test case for html text tabs

Co-authored-by: Alex Astovasadourian <asto.asto@edensoft.fr>
…ling selection of certain dates (#1862)

* (feat): add IsDateDisabled functionality

* changed prop name to follow convention

* ensure prop can never be null

* fix default prop value

Co-authored-by: Meinrad Recheis <meinrad.recheis@gmail.com>
Co-authored-by: JJ Banda <=>
-Replaced sync methods with async methods;
Copy link
Member

@HClausing HClausing left a comment

Choose a reason for hiding this comment

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

I've limited my review only on Mud Carousel components and its dependents.

About timer, if you are sure about that behavior, it's ok for me. There's some tests with AutoCycle parameter and, if they pass, that's all good.

Thanks for the bug catch on item selection, I didn't figure that clicking on the current selected item selected could cause that wrong behavior.

For the another files, there's a massive change here, I don't know if it's a good ideia to put all of this work onto a single PR, created with MudCarousel in mind. For this, I would like to request a help for @henon.

Thanks for the work!

@Yomodo
Copy link
Contributor Author

Yomodo commented Jun 18, 2021

Ouch, I didn't notice the massive change in files, focus is only on MudCarousel.

I used to merge from dev but recently learned I need to rebase from my dev branch, after updating my fork.
Thank you for this cool component!

@henon Perhaps I should close this PR en start a new one?

@henon
Copy link
Collaborator

henon commented Jun 18, 2021

@Yomodo what you can do is to set a tag so you won't lose your branch, then reset your branch --hard onto dev. Then cherry pick every commit except the merge commits. The result is like rebasing just without the merge commits which cause the trouble.

Then force push the branch.

@henon
Copy link
Collaborator

henon commented Jun 18, 2021

Or you reset --mixed to dev which will leave your changes in the working directory. Then commit as one commit to your branch and force push, effectively squashing your changes into a single commit. That is less work.

@MudBlazor MudBlazor deleted a comment from boukenka Jun 18, 2021
@MudBlazor MudBlazor deleted a comment from boukenka Jun 18, 2021
Yomodo and others added 5 commits June 18, 2021 12:00
* Refined Drawer documentation.

* Added MudVirtualize, a Virtualize which can be switched off

* Fixed MudVirtualize API test

(cherry picked from commit c8bd5d1)

* Added Table Virtualization. (#796, #1847)

* Added Table Virtualization.

* Fix formatting...

* Missing @using for virtualize

* Use MudVirtualize instead of Virtualize

* Fixed MudVirtualize API test

* Removed unused usings

Co-authored-by: Meinrad Recheis <meinrad.recheis@gmail.com>

* Autocomplete: Added Required parameter (#1499, #1829)

* Added "Required" argument to Autocomplete

* Added MudAutocomplete and MudSelect unit tests for the Required attribute.

* Refined unit tests

* Added "Required" argument to Autocomplete

* Added MudAutocomplete and MudSelect unit tests for the Required attribute.

* Refined unit tests

Co-authored-by: Jeffrey Jangli <adminjja@KPNWP.onmicrosoft.com>

* MudSelect and MudAutocomplete: Adornment improvements (#804, #912, #1659, #1835)

* Select and Autocomplete improvements

* -Select Adornment.End now variable;
-Select/Autocomplete enabled AdornmentIcon;

* merge

* -Switched MudSelect and MudAutocomplete CloseIcon <> OpenIcon values;
-

* Switched MudSelect and MudAutocomplete CurrentIcon condition

* -Docs refinment: Added AdornmentIcon, CloseIcon and OpenIcon examples for MudSelect and MudAutocomplete;
-Docs refinment: Set ShowCode="false" for Variants example;

* -Autcomplete: Added call to UpdateIcon() from IsOpen.set and removed other calls to UpdateIcon;
-Removed test app;

* MudSelect: Added @attributes="UserAttributes" to satisfy unit test.

* Made CurrentIcon private and renamed to _currentIcon

* Removed test app

* -Changed method MudAutocomplete.UpdateIcon() to private;
-Changed method MudSelect.UpdateIcon() to private;
-Changed property MudSelect.CurrentIcon to private and renamed it to _currentIcon;

* Select and Autocomplete improvements

* -Select Adornment.End now variable;
-Select/Autocomplete enabled AdornmentIcon;

* merge

* -Switched MudSelect and MudAutocomplete CloseIcon <> OpenIcon values;
-

* Switched MudSelect and MudAutocomplete CurrentIcon condition

* -Docs refinment: Added AdornmentIcon, CloseIcon and OpenIcon examples for MudSelect and MudAutocomplete;
-Docs refinment: Set ShowCode="false" for Variants example;

* -Autcomplete: Added call to UpdateIcon() from IsOpen.set and removed other calls to UpdateIcon;
-Removed test app;

* MudSelect: Added @attributes="UserAttributes" to satisfy unit test.

* Made CurrentIcon private and renamed to _currentIcon

* Removed test app

* -Changed method MudAutocomplete.UpdateIcon() to private;
-Changed method MudSelect.UpdateIcon() to private;
-Changed property MudSelect.CurrentIcon to private and renamed it to _currentIcon;

Co-authored-by: Jeffrey Jangli <adminjja@KPNWP.onmicrosoft.com>

* TabPanel: Allow HTML markup in Text (#1907, #1917)

* Add MarkupString for Tab text

* Added test case for html text tabs

Co-authored-by: Alex Astovasadourian <asto.asto@edensoft.fr>

* Docs: improved autocomplete validation example

* MudForm: improved the IsValid tow-way binding fix (previous fix was buggy)

* DatePicker and DateRangePicker: add IsDateDisabledFunc to allow disabling selection of certain dates (#1862)

* (feat): add IsDateDisabled functionality

* changed prop name to follow convention

* ensure prop can never be null

* fix default prop value

Co-authored-by: Meinrad Recheis <meinrad.recheis@gmail.com>
Co-authored-by: JJ Banda <=>

* Refined Drawer documentation.

Co-authored-by: Meinrad Recheis <meinrad.recheis@gmail.com>
Co-authored-by: Terry Phillips <terry.w.phillips@gmail.com>
Co-authored-by: Jeffrey Jangli <adminjja@KPNWP.onmicrosoft.com>
Co-authored-by: aastovas <79198906+aastovas@users.noreply.github.com>
Co-authored-by: Alex Astovasadourian <asto.asto@edensoft.fr>
Co-authored-by: JJ Banda <mtrdp642@gmail.com>
-Replaced sync methods with async methods;
-Replaced sync methods with async methods;
@Yomodo
Copy link
Contributor Author

Yomodo commented Jun 18, 2021

@henon Do I still need to do something or can this PR now be merged,
since you merged my other PR's (with the same numerous files changed)?

@henon
Copy link
Collaborator

henon commented Jun 18, 2021

In the future do not use Merge any more with feature branches. Also, you don't need to constantly rebase your PRs onto dev. When we squash merge your PR everything is automatically taken care of.

@henon henon merged commit 728093d into MudBlazor:dev Jun 18, 2021
@henon
Copy link
Collaborator

henon commented Jun 18, 2021

Thanks, the changes are all good!

@henon henon added this to the 5.0.14 milestone Jun 18, 2021
@henon henon changed the title Carousel Improvements Carousel: Changed Timer and fixed bug in MoveTo Jun 18, 2021
@Yomodo Yomodo deleted the CarouselTimer branch June 18, 2021 13:28
@HClausing
Copy link
Member

Good job @Yomodo , thanks!

@henon
Copy link
Collaborator

henon commented Jun 20, 2021

@Yomodo I just figured out that when you remove all merge commits from a feature branch by doing a reset --hard and then cherry picking all the commits (except the merge commits) and doing a force-push then the irrelevant commits completely vanish from the PR on github. Just FYI that it is possible to clean up a messed up PR.

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.

None yet

6 participants