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

refactor: get rid of Simplebar (vanilla js based scrollbar) #2900

Merged
merged 8 commits into from
Oct 3, 2024
Merged

refactor: get rid of Simplebar (vanilla js based scrollbar) #2900

merged 8 commits into from
Oct 3, 2024

Conversation

vanbasten17
Copy link
Contributor

@vanbasten17 vanbasten17 commented Sep 6, 2024 •
edited by Iru89
Loading

This PR include this other one PR#2903

Work done so far

  1. Getting rid of scrollbar webchat property customization: We don't want to support this feature anymore. This was done before in order to have "nice" styles in Internet Explorer and similar browsers. As IE is not supported anymore, we can remove this.
  2. Having ids for relevant botonic containers: In this way we can have a better way to access the values of these well differentiated areas of the webchat.
  3. Ensuring elements within webchat are spaced correctly: This is, spacing elements between in webchat for every container to push the other siblings with same distances.
  4. Remove Simplebar in favour of a new component created by us: Simplebar was giving lots of headaches to us, so we can change it by containers with the ids created in point 2.
  5. Fix replies container not scrolling
  6. chatArea container to englobe messages list and replies containers
  7. Recalculate chatarea size when iOS safari virtual keyboard is shown: As we have modified the containers which where important to calculate when the webchat has to be resized in iOS, we needed to adapt and update a little bit this logic.

Current state

As per the list described above, the functionality should be the same as before. The important thing to notice here is that WE DON'T RELY ANYMORE on simplebar selectors '.simplebar-content' and '.simplebar-content-wrapper', so bots using them should be adapted.

Next steps

  • Create reusable hooks to do calculations over DOM elements or use Resizer observer API to have the code up to date with new standards and good code practices.
  • Remove related logic not needed anymore after removing simplebar dependency (possibly tricky parts of the code related to ShadowDOM).

@vanbasten17 vanbasten17 force-pushed the refactor/update-simplebar branch 2 times, most recently from 183fe0c to b8b676e Compare September 6, 2024 14:56
@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 6, 2024
@vanbasten17 vanbasten17 marked this pull request as ready for review September 9, 2024 16:33
Iru89
Iru89 reviewed Sep 17, 2024
View reviewed changes
packages/botonic-react/src/util/dom.js Outdated Show resolved Hide resolved
packages/botonic-react/src/webchat/devices/scrollbar-controller.js Outdated Show resolved Hide resolved
packages/botonic-react/src/webchat/constants.ts Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to use ref in the components instead of ids

packages/botonic-react/src/components/carousel.jsx Show resolved Hide resolved
packages/botonic-react/src/webchat/webchat.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/webchat/webchat.jsx Outdated Show resolved Hide resolved
packages/botonic-react/src/webchat/webchat.jsx Outdated Show resolved Hide resolved
@vanbasten17 vanbasten17 mentioned this pull request Sep 19, 2024
Merged
Iru89 added a commit that referenced this pull request Oct 3, 2024
@vanbasten17 @Iru89
refactor: improvements after removing simplebar dep (#2903)
fd82e47 
This is a continuation of: #2900
Several things done here:
* Remove `scrollable-content` generic component and create it for each
scrollable div.
* Add notable refs (`webchatRef`, `chatAreaRef`, `headerRef`) in
`WebchatContext` to be accessed from different parts of the coee
* Remove JS commented code already migrated to TS.
* Reduce `util/dom.js` in favour of using new hooks.
* Migrate from JS to TS:
   * `device-adapter`
   * `scrollbar-controller`
   * `webchat-resizer`
* Simplify `WebchatHeader` component
* New hooks:
* `useScrollToBottom`: centralize logic for scrolling to the end of
scrollable messages list container
* `useVirtualKeyboardDetection`: detect whether virtual keyboard is open
or not (iOS/Safari)
* `useWebchatDimensions`: recalculation of webchat / chat area heights
* Create a new component `WebchatChatArea`
* Set textarea fontsize to 16px to be consistent with the rest of styles
and avoid auto-focusing issues on iOS.

---------

Co-authored-by: Oriol Raventos <[email protected]>
vanbasten17 and others added 8 commits October 3, 2024 10:57
@vanbasten17 @Iru89
refactor: remove scrollbar customization via webchat theme and relate…
ebf3ab1 
…d code
@vanbasten17 @Iru89
refactor: define new constant for botonic containers and use them eve…
14f46de 
…rywhere where used
@vanbasten17 @Iru89
refactor: ensure elements within webchat are spaced correctly
12e891e
@vanbasten17 @Iru89
refactor(ongoing): replace simplebar by ScrollableContent and remove dep
9d996f8
@vanbasten17 @Iru89
fix: replies container to scroll on overflow
3f6e4af
@vanbasten17 @Iru89
refactor: move chat area to main webchat file
c369ff1
@vanbasten17 @Iru89
wip: preserve functionality of resizing webchat on safari iOS, prepar…
24dd467 
…e before refactoring into hooks
@vanbasten17 @Iru89
refactor: improvements after removing simplebar dep (#2903)
64b9c00 
This is a continuation of: #2900
Several things done here:
* Remove `scrollable-content` generic component and create it for each
scrollable div.
* Add notable refs (`webchatRef`, `chatAreaRef`, `headerRef`) in
`WebchatContext` to be accessed from different parts of the coee
* Remove JS commented code already migrated to TS.
* Reduce `util/dom.js` in favour of using new hooks.
* Migrate from JS to TS:
   * `device-adapter`
   * `scrollbar-controller`
   * `webchat-resizer`
* Simplify `WebchatHeader` component
* New hooks:
* `useScrollToBottom`: centralize logic for scrolling to the end of
scrollable messages list container
* `useVirtualKeyboardDetection`: detect whether virtual keyboard is open
or not (iOS/Safari)
* `useWebchatDimensions`: recalculation of webchat / chat area heights
* Create a new component `WebchatChatArea`
* Set textarea fontsize to 16px to be consistent with the rest of styles
and avoid auto-focusing issues on iOS.

---------

Co-authored-by: Oriol Raventos <[email protected]>
@Iru89 Iru89 force-pushed the refactor/update-simplebar branch from fd82e47 to 64b9c00 Compare October 3, 2024 09:04
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 3, 2024 •
edited
Loading

Test Results

1 tests   1 ✅  2s ⏱️
1 suites  0 💤
1 files    0 ❌

Results for commit 64b9c00.

♻️ This comment has been updated with latest results.

@Iru89 Iru89 merged commit 65ab8aa into master-lts Oct 3, 2024
16 of 18 checks passed
@Iru89 Iru89 deleted the refactor/update-simplebar branch October 3, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Iru89 Iru89 Iru89 approved these changes

@ericmarcos ericmarcos Awaiting requested review from ericmarcos

@asastre asastre Awaiting requested review from asastre

Assignees
No one assigned
Labels
size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vanbasten17 @Iru89