-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Simplify ScrollSpy config #33250
Simplify ScrollSpy config #33250
Conversation
@rohit2sharma95 , @XhmikosR |
Your idea is to move the Lines 563 to 567 in 548be2e
There is also a function bootstrap/js/src/util/index.js Lines 203 to 204 in 548be2e
|
Yeah, I'm not sure about this either. We shouldn't overdo it with unnecessary abstractions. So what's the plan here exactly? |
I saw that most of them use the same functionality. So I think it could be moved as a separate function outside the component. At least for the 6 of them for the beginning. Small size improvements as keeping one pattern. (now all have different pattern of usage) |
Please finish with your suggestion so that we can see if it's worth doing it :) |
I could start standardizing a bit the configs and the interfaces, and maybe then we can discus the 'one same function'. Does this sound better? |
5df840b
to
d4a64b3
Compare
d4a64b3
to
e149302
Compare
e149302
to
ee9baa0
Compare
ee9baa0
to
0cc0109
Compare
0cc0109
to
bc6dfdf
Compare
I tried to extract a
basicJQueryInterface
to be used by other components too