Add `getOrCreateInstance` method in base-component #33276

GeoSot · 2021-03-05T22:53:29Z

   let data = Data.get(element, DATA_KEY)
  

      if (!data) {
        data = new Alert(element)
      }

can be

  const data = Alert.getOrCreateInstance(element)

rohit2sharma95

There is commented code also 😁

js/src/base-component.js

js/src/tooltip.js

GeoSot · 2021-03-07T15:07:52Z

Commented code is gone, unrelated reverted, method is renamed :)

XhmikosR · 2021-03-08T16:44:48Z

Would this conflict with #33249 and #33250? If so, unsure which should land first.

BTW we don't really test jQuery, just if it works, so I'm a little hesitant :/

GeoSot · 2021-03-08T19:43:09Z

The other two #33249 and #33250, are small easy refactorings , so you can land them first.

As for the jQuery. Every one of them, does not import something new. Only it streamlines the same code snippet

getInstance or make new if config is obj

It is based on the existing tests for Jquery

js/src/collapse.js

js/src/popover.js

js/src/tooltip.js

alecpl · 2021-04-16T05:46:10Z

It would be great if all these @GeoSot's refactoring PRs could make it to 5.0.0. @mdo any chance for that?

@GeoSot is this still a draft? looks like some tests failed.

XhmikosR · 2021-04-16T05:49:36Z

I'd rather not get the further refactoring PRs in v5.0.0 to not risk to break something so close to the release. There's plenty of time in 5.0.1 or 5.1.0.

GeoSot · 2021-04-16T07:35:45Z

stable 5.0.0 is the most important thing for now. The next thing, I bet are some of the refactorings

GeoSot · 2021-05-31T12:20:54Z

I have added the proper documentation. So do we need something more in order to proceed?

js/src/offcanvas.js

js/tests/unit/alert.spec.js

js/tests/unit/offcanvas.spec.js

site/content/docs/5.0/components/alerts.md

…lready exists Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com>

alpadev

LGTM, from my side.

rohit2sharma95

/CC @mdo for the documentation, if required

js/tests/unit/base-component.spec.js

rohit2sharma95 · 2021-06-02T10:23:35Z

js/tests/unit/tooltip.spec.js

@@ -3,7 +3,7 @@ import EventHandler from '../../src/dom/event-handler'
 import { noop } from '../../src/util/index'

 /** Test helpers */
-import { getFixture, clearFixture, jQueryMock, createEvent } from '../helpers/fixture'
+import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'


You must be using any code formatter in your IDE 😅

HAHAHA jetBrains like them sorted 😋

js/src/offcanvas.js

Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com>

Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com> Co-authored-by: XhmikosR <xhmikosr@gmail.com>

GeoSot requested a review from a team as a code owner March 5, 2021 22:53

GeoSot changed the title ~~initial~~ Add 'getInstanceOrNew' method in base-component Mar 5, 2021

rohit2sharma95 reviewed Mar 7, 2021

View reviewed changes

js/src/base-component.js Outdated Show resolved Hide resolved

js/src/tooltip.js Outdated Show resolved Hide resolved

rohit2sharma95 added js v5 labels Mar 7, 2021

GeoSot force-pushed the get-instance-or-new branch from 3e188d8 to 6756bd5 Compare March 9, 2021 18:00

XhmikosR reviewed Mar 10, 2021

View reviewed changes

js/src/collapse.js Outdated Show resolved Hide resolved

GeoSot force-pushed the get-instance-or-new branch from 6756bd5 to c53ac10 Compare March 10, 2021 08:31

XhmikosR reviewed Mar 10, 2021

View reviewed changes

js/src/popover.js Outdated Show resolved Hide resolved

XhmikosR reviewed Mar 10, 2021

View reviewed changes

js/src/tooltip.js Outdated Show resolved Hide resolved

GeoSot force-pushed the get-instance-or-new branch 2 times, most recently from 6d6c8fe to e64567f Compare March 16, 2021 21:54

GeoSot marked this pull request as draft March 17, 2021 23:17

GeoSot added the refactoring label Mar 23, 2021

GeoSot force-pushed the get-instance-or-new branch 2 times, most recently from c9a751c to a1aeb58 Compare March 23, 2021 23:27

GeoSot force-pushed the get-instance-or-new branch from a1aeb58 to d4ed6fa Compare April 6, 2021 22:26

GeoSot force-pushed the get-instance-or-new branch from d4ed6fa to 2f5e05a Compare April 14, 2021 23:21

GeoSot force-pushed the get-instance-or-new branch 2 times, most recently from 60f8aff to 6ecb5fd Compare May 11, 2021 08:32

GeoSot force-pushed the get-instance-or-new branch 2 times, most recently from 0be5583 to ce08bb5 Compare May 18, 2021 23:39

GeoSot force-pushed the get-instance-or-new branch 2 times, most recently from c11b060 to 8ca1f63 Compare May 20, 2021 13:38

alpadev reviewed Jun 2, 2021

View reviewed changes

js/src/offcanvas.js Outdated Show resolved Hide resolved

alpadev reviewed Jun 2, 2021

View reviewed changes

js/tests/unit/alert.spec.js Show resolved Hide resolved

alpadev reviewed Jun 2, 2021

View reviewed changes

js/tests/unit/offcanvas.spec.js Show resolved Hide resolved

alpadev reviewed Jun 2, 2021

View reviewed changes

site/content/docs/5.0/components/alerts.md Outdated Show resolved Hide resolved

GeoSot and others added 6 commits June 2, 2021 11:52

Base-Component: Make static method getOrCreateInstance

cfe7336

fix dropdown leftovers

994d1c4

add tests on base-component

6bce805

Prevent config variable check in base-component.js, if the instance a…

aad576d

…lready exists Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com>

Add documentation for the new method

cfcf88b

apply minor changes after review

4e7acbe

GeoSot force-pushed the get-instance-or-new branch from 89d1636 to 4e7acbe Compare June 2, 2021 09:06

alpadev approved these changes Jun 2, 2021

View reviewed changes

rohit2sharma95 approved these changes Jun 2, 2021

View reviewed changes

GeoSot and others added 7 commits June 2, 2021 13:56

Remove extra parentheses on js/src/offcanvas.js

cf6a7cb

Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com>

Update buttons.md

61538d4

Update buttons.md

bd4a70c

Update carousel.md

1a42f7d

Update collapse.md

dc46be1

Update dropdowns.md

1cfaaac

Merge branch 'main' into get-instance-or-new

2bf37da

XhmikosR changed the title ~~Add 'getOrCreateInstance' method in base-component~~ Add getOrCreateInstance method in base-component Jun 3, 2021

XhmikosR merged commit c98657b into twbs:main Jun 3, 2021

GeoSot deleted the get-instance-or-new branch June 3, 2021 22:32

alpadev mentioned this pull request Jun 15, 2021

Carousel controls broken in main #34226

Closed

marvin-hinkley-vortx pushed a commit to Vortx-Inc/bootstrap that referenced this pull request Aug 18, 2021

Add getOrCreateInstance method in base-component (twbs#33276)

96ea07b

Co-authored-by: Rohit Sharma <rohit2sharma95@gmail.com> Co-authored-by: XhmikosR <xhmikosr@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `getOrCreateInstance` method in base-component #33276

Add `getOrCreateInstance` method in base-component #33276

GeoSot commented Mar 5, 2021 •

edited

Loading

rohit2sharma95 left a comment

GeoSot commented Mar 7, 2021

XhmikosR commented Mar 8, 2021

GeoSot commented Mar 8, 2021 •

edited

Loading

alecpl commented Apr 16, 2021

XhmikosR commented Apr 16, 2021

GeoSot commented Apr 16, 2021

GeoSot commented May 31, 2021

alpadev left a comment

rohit2sharma95 left a comment

rohit2sharma95 Jun 2, 2021

GeoSot Jun 2, 2021

Add getOrCreateInstance method in base-component #33276

Add getOrCreateInstance method in base-component #33276

Conversation

GeoSot commented Mar 5, 2021 • edited Loading

rohit2sharma95 left a comment

Choose a reason for hiding this comment

GeoSot commented Mar 7, 2021

XhmikosR commented Mar 8, 2021

GeoSot commented Mar 8, 2021 • edited Loading

alecpl commented Apr 16, 2021

XhmikosR commented Apr 16, 2021

GeoSot commented Apr 16, 2021

GeoSot commented May 31, 2021

alpadev left a comment

Choose a reason for hiding this comment

rohit2sharma95 left a comment

Choose a reason for hiding this comment

rohit2sharma95 Jun 2, 2021

Choose a reason for hiding this comment

GeoSot Jun 2, 2021

Choose a reason for hiding this comment

Add `getOrCreateInstance` method in base-component #33276

Add `getOrCreateInstance` method in base-component #33276

GeoSot commented Mar 5, 2021 •

edited

Loading

GeoSot commented Mar 8, 2021 •

edited

Loading