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

Jump to bottom

Remove .show from panels dependent-less on the existence of .fade class #36622

Merged
merged 2 commits into from Jun 27, 2022
Merged

Remove .show from panels dependent-less on the existence of .fade class #36622

merged 2 commits into from Jun 27, 2022

Conversation

GeoSot
Copy link
Member

@GeoSot GeoSot commented Jun 22, 2022 •
edited

closes #36621

@GeoSot GeoSot marked this pull request as ready for review June 22, 2022 08:39
@GeoSot GeoSot requested a review from a team as a code owner June 22, 2022 08:39
@GeoSot GeoSot changed the title Remove .show from panels depends less on the existance of .fade class Remove .show from panels dependent-less on the existence of .fade class Jun 22, 2022
@GeoSot GeoSot added this to In progress in v5.2.0-stable via automation Jun 22, 2022
@GeoSot GeoSot added this to In progress in v5.3.0 via automation Jun 22, 2022
@julien-deramond
Copy link
Member

CodePen mentioned in the issue is based on bootstrap@latest (5.1.3). When modifying this CodePen to use bootstrap@5.2.0-beta1, I can't see the issue anymore.
So do we still need the modification embedded in this PR because it is safer like that?

@GeoSot
Copy link
Member Author

GeoSot commented Jun 25, 2022

During the latest big refactoring, this check has been left same as previous, in order to minimize the changes on tests (and I had added the todo comment, knowing this probably would better to be changed)

I believe it's better on this generic way

julien-deramond
julien-deramond approved these changes Jun 25, 2022
View reviewed changes
Copy link
Member

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

v5.2.0-stable automation moved this from In progress to Reviewer approved Jun 25, 2022
@GeoSot GeoSot merged commit f14c58f into main Jun 27, 2022
v5.3.0 automation moved this from In progress to Done Jun 27, 2022
@GeoSot GeoSot deleted the gs/tabs-ref branch June 27, 2022 09:43
v5.2.0-stable automation moved this from Reviewer approved to Done Jun 27, 2022
@GeoSot GeoSot removed this from Done in v5.3.0 Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julien-deramond julien-deramond julien-deramond approved these changes

Assignees
No one assigned
Labels
js v5
Projects
No open projects
v5.2.0-stable
  
Done
v5.2.0-stable
Status: Done
Milestone
No milestone
2 participants
@GeoSot @julien-deramond