Skip to content

fix(footer): remove toolbar bottom padding if near bottom slot tabs or keyboard is open#25746

Merged
averyrousseau merged 20 commits into
mainfrom
FW-1925
Aug 16, 2022
Merged

fix(footer): remove toolbar bottom padding if near bottom slot tabs or keyboard is open#25746
averyrousseau merged 20 commits into
mainfrom
FW-1925

Conversation

@averyrousseau
Copy link
Copy Markdown
Contributor

@averyrousseau averyrousseau commented Aug 10, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
    • Some docs updates need to be made in the ionic-docs repo, in a separate PR. See the contributing guide for details.
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Currently, an ion-toolbar inside an ion-footer receives bottom padding according to the safe area, via this CSS. However, this padding becomes unnecessary if there is also an ion-tab-bar on the bottom of the screen, as it also has its own safe area bottom padding.

extra padding

This situation also applies when you have the phone’s keyboard open, with or without an ion-tab-bar.

Issue URL: Resolves #17560

What is the new behavior?

Extra bottom padding removed if there is a slot="bottom" tab bar nearby, or if the keyboard is open. (The keyboard logic is the same as in ion-tab-bar.)

This PR also pulls the keyboardVisible logic shared between ion-footer and ion-tab-bar into a separate createKeyboardController function.

Does this introduce a breaking change?

  • Yes
  • No

Other information

This PR is adapted from #23193. Co-author credit will be given when merging.

I left off a test for the keyboard being open since I don't believe that's possible in Playwright 🤔

…rd is open

Co-authored-by: EinfachHans <EinfachHans@users.noreply.github.com>
@github-actions github-actions Bot added the package: core @ionic/core package label Aug 10, 2022
@averyrousseau averyrousseau changed the title fix(footer): remove bottom padding if near bottom slot tabs or keyboard is open fix(footer): remove toolbar bottom padding if near bottom slot tabs or keyboard is open Aug 10, 2022
@averyrousseau averyrousseau marked this pull request as ready for review August 10, 2022 18:08
@averyrousseau averyrousseau requested a review from a team August 10, 2022 18:08
Comment thread core/src/components/footer/footer.tsx Outdated
Comment thread core/src/components/footer/footer.tsx Outdated
Comment thread core/src/components/footer/footer.tsx Outdated
Comment thread core/src/components/footer/footer.tsx Outdated
@sean-perkins
Copy link
Copy Markdown
Contributor

sean-perkins commented Aug 10, 2022

One possible approach to sharing the logic across both implementations would be through a controller, similar to how we handle modal, popover, menu, etc.

const createKeyboardController = () => {
  let keyboardWillShowHandler: (() => void) | undefined;
  let keyboardWillHideHandler: (() => void) | undefined;
  let keyboardVisible: boolean;

  const init = (el?: HTMLElement) => {
    keyboardWillShowHandler = () => {
      if (el && el.getAttribute('slot') !== 'top') {
        keyboardVisible = true;
      }
    };

    keyboardWillHideHandler = () => {
      setTimeout(() => (keyboardVisible = false), 50);
    };

    window.addEventListener('keyboardWillShow', keyboardWillShowHandler!);
    window.addEventListener('keyboardWillHide', keyboardWillHideHandler!);
  }

  const destroy = () => {
    if (typeof window !== 'undefined') {
      window.removeEventListener('keyboardWillShow', keyboardWillShowHandler!);
      window.removeEventListener('keyboardWillHide', keyboardWillHideHandler!);

      keyboardWillShowHandler = keyboardWillHideHandler = undefined;
    }
  }

  const isKeyboardVisible = () => keyboardVisible;

  return { init, destroy, isKeyboardVisible };
}
private keyboardCtrl: any; // TODO type

connectedCallback() {
  this.keyboardCtrl = createKeyboardController();
  this.keyboardCtrl.init(this.el);
}

disconnectedCallback() {
  if (this.keyboardCtrl) {
    this.keyboardCtrl.destroy();
  }
}

In your implementation you need to trigger a re-render when the keyboard visibility changes. For this, you may need to pass in a callback that can trigger your state variable to re-render.

Comment thread core/src/utils/keyboard/keyboard-controller.ts Outdated
Comment thread core/src/components/footer/footer.tsx Outdated
Comment thread core/src/components/footer/test/with-tabs/footer.e2e.ts
Comment thread core/src/components/tab-bar/tab-bar.tsx Outdated
Comment thread core/src/components/footer/footer.tsx
Comment thread core/src/components/footer/test/with-tabs/footer.e2e.ts
Comment thread core/src/components/footer/footer.tsx
Comment thread core/src/utils/keyboard/keyboard-controller.ts Outdated
Copy link
Copy Markdown
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Great job! This is good to go once tests for the keyboard controller are written.

Comment thread core/src/utils/keyboard/keyboard-controller.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Footer toolbar should not have safe-area-inset-bottom with tab-bar active

4 participants