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

_unregisterItem of ProviderParentMixin won't work if child items do not have value #194

Closed
kikuomax opened this issue Feb 16, 2024 · 4 comments · Fixed by #196
Closed

_unregisterItem of ProviderParentMixin won't work if child items do not have value #194

kikuomax opened this issue Feb 16, 2024 · 4 comments · Fixed by #196
Assignees
@kikuomax
Labels
bug Something isn't working internal Related to internal feature
Milestone
v0.1.3

Comments

@kikuomax
Copy link
Collaborator

Overview of the problem

Buefy version: [0.1.2]
Vuejs version: [3.x]
OS/Browser: should not matter

Description

See

buefy-next/packages/buefy-next/src/utils/ProviderParentMixin.js

Lines 37 to 39 in 20675f1

_unregisterItem(item) {
this.childItems = this.childItems.filter((i) => i.value !== item.value)
}

Steps to reproduce

Expected behavior

_unregisterItem should work if child items lack value.

Actual behavior

_untegisterItem won't work if child items lack value.
@kikuomax kikuomax added bug Something isn't working need investigation Need investigation labels Feb 16, 2024
@kikuomax kikuomax changed the title _unregisterItem of ProviderParentMixin won't work child items do not have value _unregisterItem of ProviderParentMixin won't work if child items do not have value Feb 16, 2024
@kikuomax kikuomax added the internal Related to internal feature label Feb 16, 2024
@kikuomax
Copy link
Collaborator Author

kikuomax commented Feb 16, 2024 •
edited

ProviderParentMixin expects children mixing in InjectedChildMixin. Users of InjectedChildMixin:

  • CarouselItem: no value
  • DropdownItem: value: [String, Number, Boolean, Object, Array, Function]
  • ProgressBar: value: Number
  • TabbedChildMixin: value: String
    Used by:
    • StepItem
    • TabItem

@kikuomax kikuomax mentioned this issue Feb 16, 2024
Closed
@kikuomax
Copy link
Collaborator Author

kikuomax commented Feb 20, 2024 •
edited

I think ProgressBar should stop using InjectedChildMixin because its parent Progress does not use any functionalities of ProviderParentMixin. Providing Progress instance to ProgressBar is sufficient.

@kikuomax
Copy link
Collaborator Author

DropdownItem neither has to use InjectedChildMixin because its parent Dropdown does not use any functionalities of ProviderParentMixin. Providing Dropdown instance to DropdownItem is sufficient.

@kikuomax
Copy link
Collaborator Author

_unregisterItem won't work for CarouselItem due to missing value.

@kikuomax kikuomax mentioned this issue Feb 22, 2024
Merged
@kikuomax kikuomax self-assigned this Feb 22, 2024
@kikuomax kikuomax added this to the v0.1.3 milestone Feb 22, 2024
@kikuomax kikuomax removed the need investigation Need investigation label Feb 27, 2024
kikuomax added a commit that referenced this issue Mar 8, 2024
@kikuomax
Fix: BTabs does not work in SSR due to window access (#187) and relat…
53f9b25 
…ed issue (#194) (#196)

* feat(lib): make Dropdown stop using ProviderParentMixin

- `Dropdown` stops mixing in `ProviderParentMixin` but just `provide`s
  its instance associated with `DROPDOWN_INJECTION_KEY` because
  `Dropdown` does not use any funcitionalities of `ProviderParentMixin`.
- `DropdownItem` stops mixing in `InjectedChildMixin` but just `inject`s
  the instance of `Dropdown` associated with `DROPDOWN_INJECTION_KEY`.
- `DROPDOWN_INJECTION_KEY` is a unique `Symbol`.
- Updates also the spec.

* feat(lib): make Progress stop using ProviderParentMixin

- `Progress` stops mixing in `ProviderParentMixin` but `provide`s its
  instance associated with `PROGRESS_INJECTION_KEY` because `Progress`
  does not use any functionalities of `ProviderParentMixin`.
- `ProgressBar` stops mixin in `InjectedChildMixin` but `inject`s the
  instance of `Progress` associated with `PROGRESS_INJECTION_KEY`.
- `PROGRESS_INJECTION_KEY` is a unique `Symbol`.
- Updates the spec for `ProgressBar`. It also introduces a new test case
  that tests `ProgressBar` reads `max` from the parent `Progress`.
  Refreshes the spec snapshot.

* fix(lib): BTabs did not work in SSR, and BCarousel

- The issue that `BTabs` did not work in Server Side Rendering (SSR)
  due to `window` access. `window` was necessary to generate a random
  number to initialize `value` prop of `BTabItem` with a unique value
  when `value` is omitted. Before Vue 3 migration, it used to use
  `this._uid` internal field to initialze `value`. However, `this` is no
  longer available during the prop initialization on Vue 3. So I came up
  with the idea of random numbers. Since random numbers also won't work
  well with SSR, we had to give up random numbers anyway. A new approach
  is to avoid direct use of `value` prop but computed `uniqueValue`
  which becomes `value` if it is non-null, otherwise `this.$.uid`
  internal field. Definitions of `value` and `uniqueValue` are hoisted
  to `InjectedChildMixin` which `BTabs` mixes in through
  `TabbedChildMixin` because there was another problem with
  `CarouselItem` (see below).

- `CarouselItem` is another component that mixes in
  `InjectedChildMixin`. There was a bug that `CarouselItem` was not able
  to be correctly removed from the parent `Carousel`. This bug is also
  fixed by defining `value` and `uniqueValue` in `InjectedChildMixin`.

- `ProviderParentMixin`, `TabbedMixin`, `TabbedChildMixin`, `Tabs`,
  `Steps`, and `CarouselItem` use `uniqueValue` instead of `value`
  whenever the identity of the child matters.

- `Carousel` introduces `activeChildIndex` that `CarouselItem` uses to
  determine the item is active, because `activeChild` of `Carousel` is
  an index in the array that `Carousel` locally holds and may not match
  the `index` that `CarouselItem` recognizes.

- `utils/make-unique-id.js` no longer necessary.

- Introduces new test cases for `TabItem`, `StepItem`, and
  `CarouselItem` that make sure `uniqueValue` and `order` work.
  Refreshes the spec snapshot of `CarouselItem`.

* feat(docs): add CarouselItem show/hide

- `ExFull` example for `Carousel` introduces a new switch to show/hide a
  certain slide in `Carousel`. This makes sure that `order` prop of
  `CarouselItem` works.

* style(docs): fix lint error

* feat(docs): introduce order prop

- Examples of `Carousel`, `Steps`, and `Tabs` explain how to use `order`
  prop.

* docs: update CHANGELOG

- `CHANGELOG` explains `order` prop of `CarouselItem`, `StepItem`, and
  `TabItem`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kikuomax kikuomax

Labels
bug Something isn't working internal Related to internal feature
Projects
Buefy Vue3 Project Board
Status: ✅ Done
Milestone
v0.1.3
Development

Successfully merging a pull request may close this issue.

Fix: BTabs does not work in SSR due to window access (#187) and related issue (#194) kikuomax/buefy
1 participant
@kikuomax