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.

Sign up for GitHub

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

Show multiple items when initialize #40

Merged
merged 5 commits into from
Jun 20, 2023
Merged

Show multiple items when initialize #40

merged 5 commits into from
Jun 20, 2023

Conversation

vuquangpham
Copy link
Contributor

Problem

  • We are unable to open multiple sections (activeSections) in the array.

Why

image
We have a condition for blocking to avoid the double click on the animating item, but we need to escape that condition if we input activeSections in the array by default

Problem

  • Add hasActivated flag for checking it in getToggleState function.

@vuquangpham
fix(active-section): add activatedActiveSection flag for checking ani…
281439a 
…mating status to open multiple active section in default
@vuquangpham vuquangpham added the bug Something isn't working label Apr 13, 2023
@vuquangpham vuquangpham requested a review from phucbm April 13, 2023 08:34
@vuquangpham vuquangpham self-assigned this Apr 13, 2023
@netlify
Copy link

netlify bot commented Apr 13, 2023 •
edited
Loading

✅ Deploy Preview for easy-tab-accordion ready!

Name Link
🔨 Latest commit 281439a
🔍 Latest deploy log https://app.netlify.com/sites/easy-tab-accordion/deploys/6437be9e3741d80007803ee0
😎 Deploy Preview https://deploy-preview-40--easy-tab-accordion.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 settings.

@phucbm
Copy link
Member

phucbm commented Apr 17, 2023

We are unable to open multiple sections (activeSections) in the array.

  1. Could you link the source code that you're talking about here? And why should we need to be able to "open multiple sections"?

but we need to escape that condition if we input activeSections in the array by default

  1. Why do we need this?

I don't understand what are you trying to achieve here!

@phucbm
Copy link
Member

phucbm commented May 8, 2023

Please merge this branch into branch 2.1.7.

phucbm
phucbm reviewed May 15, 2023
View reviewed changes
src/_index.js Outdated
@@ -150,6 +153,8 @@ export class EasyTabAccordion{
}
}

this.hasActivatedSection = true;
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of line 145 when you override the value immediately here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we have checked it inside this block of code

image

image

phucbm
phucbm requested changes May 15, 2023
View reviewed changes
Copy link
Member

@phucbm phucbm left a comment

Choose a reason for hiding this comment

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

Please check my comments.

@vuquangpham
Copy link
Contributor Author

We are unable to open multiple sections (activeSections) in the array.

  1. Could you link the source code that you're talking about here? And why should we need to be able to "open multiple sections"?

but we need to escape that condition if we input activeSections in the array by default

  1. Why do we need this?

I don't understand what are you trying to achieve here!

Because we already have these options but we can't use it
image

@phucbm
Copy link
Member

phucbm commented May 17, 2023

@vuquangpham I was talking about this one #40 (comment)

@phucbm
Copy link
Member

phucbm commented Jun 19, 2023

@vuquangpham Any updates?

@vuquangpham vuquangpham changed the base branch from main to 2.1.7 June 20, 2023 02:39
@vuquangpham
Copy link
Contributor Author

vuquangpham commented Jun 20, 2023 •
edited
Loading

@phucbm
I have renamed the variable. Could you please check it again? Many thanks.

phucbm
phucbm approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@phucbm phucbm left a comment

Choose a reason for hiding this comment

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

LGTM.

@vuquangpham vuquangpham changed the title Add hasActivatedSection flag Show multiple items when initialize Jun 20, 2023
@phucbm phucbm merged commit b108b3c into 2.1.7 Jun 20, 2023
@phucbm phucbm deleted the default-active-sections branch June 20, 2023 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phucbm phucbm phucbm approved these changes

Assignees

@vuquangpham vuquangpham

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vuquangpham @phucbm