Skip to content

Commit

Permalink
fix(ios): large title transition accounts for back button with no text (
Browse files Browse the repository at this point in the history
#29327)

Issue number: resolves #28751

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

The large title transition does not account for back buttons with no
text value. We assume that the [`.button-text` element
](https://github.com/ionic-team/ionic-framework/blob/bfaf528e61fd82c9106e3c5060921fb79d97156a/core/src/components/back-button/back-button.tsx#L168)
is always defined, but that is not the case when `text=""` on the back
button. As a result, devs were getting errors because we tried to get
the bounding box of a undefined.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Revised the large title logic to only grab values from the back button
text if the back button text element is actually defined

There should be **no behavior change** when the back button text element
is defined.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev Build: `7.8.5-dev.11713282122.18cabf98`

⚠️ Reviewers: Please test this in the sample application on the linked
issue. Please be sure to test the following conditions:

1. When the back button text is defined
2. When the back button text is not defined
3. With the default font scale
4. With a larger font scale
  • Loading branch information
liamdebeasi authored Apr 30, 2024
1 parent 4d09890 commit bd8d065
Showing 1 changed file with 110 additions and 77 deletions.
187 changes: 110 additions & 77 deletions core/src/utils/transition/ios.transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ const createLargeTitleTransition = (
const leavingLargeTitleBox = leavingLargeTitle.getBoundingClientRect();
const enteringBackButtonBox = enteringBackButton.getBoundingClientRect();

const enteringBackButtonTextEl = shadow(enteringBackButton).querySelector('.button-text')!;
const enteringBackButtonTextBox = enteringBackButtonTextEl.getBoundingClientRect();
const enteringBackButtonTextEl = shadow(enteringBackButton).querySelector('.button-text');

// Text element not rendered if developers pass text="" to the back button
const enteringBackButtonTextBox = enteringBackButtonTextEl?.getBoundingClientRect();

const leavingLargeTitleTextEl = shadow(leavingLargeTitle).querySelector('.toolbar-title')!;
const leavingLargeTitleTextBox = leavingLargeTitleTextEl.getBoundingClientRect();
Expand All @@ -88,6 +90,7 @@ const createLargeTitleTransition = (
leavingLargeTitle,
leavingLargeTitleBox,
leavingLargeTitleTextBox,
enteringBackButtonBox,
enteringBackButtonTextEl,
enteringBackButtonTextBox
);
Expand All @@ -106,8 +109,10 @@ const createLargeTitleTransition = (
const enteringLargeTitleBox = enteringLargeTitle.getBoundingClientRect();
const leavingBackButtonBox = leavingBackButton.getBoundingClientRect();

const leavingBackButtonTextEl = shadow(leavingBackButton).querySelector('.button-text')!;
const leavingBackButtonTextBox = leavingBackButtonTextEl.getBoundingClientRect();
const leavingBackButtonTextEl = shadow(leavingBackButton).querySelector('.button-text');

// Text element not rendered if developers pass text="" to the back button
const leavingBackButtonTextBox = leavingBackButtonTextEl?.getBoundingClientRect();

const enteringLargeTitleTextEl = shadow(enteringLargeTitle).querySelector('.toolbar-title')!;
const enteringLargeTitleTextBox = enteringLargeTitleTextEl.getBoundingClientRect();
Expand All @@ -119,6 +124,7 @@ const createLargeTitleTransition = (
enteringLargeTitle,
enteringLargeTitleBox,
enteringLargeTitleTextBox,
leavingBackButtonBox,
leavingBackButtonTextEl,
leavingBackButtonTextBox
);
Expand Down Expand Up @@ -147,8 +153,8 @@ const animateBackButton = (
backDirection: boolean,
backButtonEl: HTMLIonBackButtonElement,
backButtonBox: DOMRect,
backButtonTextEl: HTMLElement,
backButtonTextBox: DOMRect,
backButtonTextEl: HTMLElement | null,
backButtonTextBox: DOMRect | undefined,
largeTitleEl: HTMLIonTitleElement,
largeTitleTextBox: DOMRect
) => {
Expand All @@ -158,31 +164,35 @@ const animateBackButton = (
const ICON_ORIGIN_X = rtl ? 'left' : 'right';

const CONTAINER_ORIGIN_X = rtl ? 'right' : 'left';
let WIDTH_SCALE = 1;
let HEIGHT_SCALE = 1;

/**
* When the title and back button texts match
* then they should overlap during the page transition.
* If the texts do not match up then the back button text scale adjusts
* to not perfectly match the large title text otherwise the
* proportions will be incorrect.
* When the texts match we scale both the width and height to account for
* font weight differences between the title and back button.
*/
const doTitleAndButtonTextsMatch = backButtonTextEl.textContent?.trim() === largeTitleEl.textContent?.trim();

const WIDTH_SCALE = largeTitleTextBox.width / backButtonTextBox.width;

/**
* We subtract an offset to account for slight sizing/padding
* differences between the title and the back button.
*/
const HEIGHT_SCALE = (largeTitleTextBox.height - LARGE_TITLE_SIZE_OFFSET) / backButtonTextBox.height;

const TEXT_START_SCALE = doTitleAndButtonTextsMatch
? `scale(${WIDTH_SCALE}, ${HEIGHT_SCALE})`
: `scale(${HEIGHT_SCALE})`;
let TEXT_START_SCALE = `scale(${HEIGHT_SCALE})`;
const TEXT_END_SCALE = 'scale(1)';

if (backButtonTextEl && backButtonTextBox) {
/**
* When the title and back button texts match then they should overlap during the
* page transition. If the texts do not match up then the back button text scale
* adjusts to not perfectly match the large title text otherwise the proportions
* will be incorrect. When the texts match we scale both the width and height to
* account for font weight differences between the title and back button.
*/
const doTitleAndButtonTextsMatch = backButtonTextEl.textContent?.trim() === largeTitleEl.textContent?.trim();
WIDTH_SCALE = largeTitleTextBox.width / backButtonTextBox.width;
/**
* Subtract an offset to account for slight sizing/padding differences between the
* title and the back button.
*/
HEIGHT_SCALE = (largeTitleTextBox.height - LARGE_TITLE_SIZE_OFFSET) / backButtonTextBox.height;

/**
* Even though we set TEXT_START_SCALE to HEIGHT_SCALE above, we potentially need
* to re-compute this here since the HEIGHT_SCALE may have changed.
*/
TEXT_START_SCALE = doTitleAndButtonTextsMatch ? `scale(${WIDTH_SCALE}, ${HEIGHT_SCALE})` : `scale(${HEIGHT_SCALE})`;
}

const backButtonIconEl = shadow(backButtonEl).querySelector('ion-icon')!;
const backButtonIconBox = backButtonIconEl.getBoundingClientRect();

Expand Down Expand Up @@ -292,12 +302,11 @@ const animateBackButton = (
top: '0px',
[CONTAINER_ORIGIN_X]: '0px',
})
.keyframes(CONTAINER_KEYFRAMES);

enteringBackButtonTextAnimation
.beforeStyles({
'transform-origin': `${TEXT_ORIGIN_X} top`,
})
/**
* The write hooks must be set on this animation as it is guaranteed to run. Other
* animations such as the back button text animation will not run if the back button
* has no visible text.
*/
.beforeAddWrite(() => {
backButtonEl.style.setProperty('display', 'none');
clonedBackButtonEl.style.setProperty(TEXT_ORIGIN_X, BACK_BUTTON_START_OFFSET);
Expand All @@ -307,6 +316,12 @@ const animateBackButton = (
clonedBackButtonEl.style.setProperty('display', 'none');
clonedBackButtonEl.style.removeProperty(TEXT_ORIGIN_X);
})
.keyframes(CONTAINER_KEYFRAMES);

enteringBackButtonTextAnimation
.beforeStyles({
'transform-origin': `${TEXT_ORIGIN_X} top`,
})
.keyframes(TEXT_KEYFRAMES);

enteringBackButtonIconAnimation
Expand All @@ -329,8 +344,9 @@ const animateLargeTitle = (
largeTitleEl: HTMLIonTitleElement,
largeTitleBox: DOMRect,
largeTitleTextBox: DOMRect,
backButtonTextEl: HTMLElement,
backButtonTextBox: DOMRect
backButtonBox: DOMRect,
backButtonTextEl: HTMLElement | null,
backButtonTextBox: DOMRect | undefined
) => {
/**
* The horizontal transform origin for the large title
Expand All @@ -353,59 +369,76 @@ const animateLargeTitle = (
* title and the back button due to padding and font weight.
*/
const LARGE_TITLE_TRANSLATION_OFFSET = 8;
let END_TRANSLATE_X = rtl
? `-${window.innerWidth - backButtonBox.right - LARGE_TITLE_TRANSLATION_OFFSET}px`
: `${backButtonBox.x + LARGE_TITLE_TRANSLATION_OFFSET}px`;

/**
* The scaled title should (roughly) overlap the back button.
* This ensures that the back button and title overlap during
* the animation. Note that since both elements either fade in
* or fade out over the course of the animation, neither element
* will be fully visible on top of the other. As a result, the overlap
* does not need to be perfect, so approximate values are acceptable here.
* How much to scale the large title up/down by.
*/
const END_TRANSLATE_X = rtl
? `-${window.innerWidth - backButtonTextBox.right - LARGE_TITLE_TRANSLATION_OFFSET}px`
: `${backButtonTextBox.x - LARGE_TITLE_TRANSLATION_OFFSET}px`;
let HEIGHT_SCALE = 0.5;

/**
* The top of the scaled large title
* should match with the top of the
* back button text element.
* We subtract 2px to account for the top padding
* on the large title element.
* The large title always starts full size.
*/
const LARGE_TITLE_TOP_PADDING = 2;
const END_TRANSLATE_Y = `${backButtonTextBox.y - LARGE_TITLE_TOP_PADDING}px`;
const START_SCALE = 'scale(1)';

/**
* In the forward direction, the large title should start at its
* normal size and then scale down to be (roughly) the size of the
* back button on the other view. In the backward direction, the
* large title should start at (roughly) the size of the back button
* and then scale up to its original size.
*
* Note that since both elements either fade in
* or fade out over the course of the animation, neither element
* will be fully visible on top of the other. As a result, the overlap
* does not need to be perfect, so approximate values are acceptable here.
* By default, we don't worry about having the large title scaled to perfectly
* match the back button because we don't know if the back button's text matches
* the large title's text.
*/
let END_SCALE = `scale(${HEIGHT_SCALE})`;

// Text element not rendered if developers pass text="" to the back button
if (backButtonTextEl && backButtonTextBox) {
/**
* The scaled title should (roughly) overlap the back button. This ensures that
* the back button and title overlap during the animation. Note that since both
* elements either fade in or fade out over the course of the animation, neither
* element will be fully visible on top of the other. As a result, the overlap
* does not need to be perfect, so approximate values are acceptable here.
*/
END_TRANSLATE_X = rtl
? `-${window.innerWidth - backButtonTextBox.right - LARGE_TITLE_TRANSLATION_OFFSET}px`
: `${backButtonTextBox.x - LARGE_TITLE_TRANSLATION_OFFSET}px`;

/**
* In the forward direction, the large title should start at its normal size and
* then scale down to be (roughly) the size of the back button on the other view.
* In the backward direction, the large title should start at (roughly) the size
* of the back button and then scale up to its original size.
* Note that since both elements either fade in or fade out over the course of the
* animation, neither element will be fully visible on top of the other. As a result,
* the overlap does not need to be perfect, so approximate values are acceptable here.
*/

/**
* When the title and back button texts match then they should overlap during the
* page transition. If the texts do not match up then the large title text scale
* adjusts to not perfectly match the back button text otherwise the proportions
* will be incorrect. When the texts match we scale both the width and height to
* account for font weight differences between the title and back button.
*/
const doTitleAndButtonTextsMatch = backButtonTextEl.textContent?.trim() === largeTitleEl.textContent?.trim();

const WIDTH_SCALE = backButtonTextBox.width / largeTitleTextBox.width;
HEIGHT_SCALE = backButtonTextBox.height / (largeTitleTextBox.height - LARGE_TITLE_SIZE_OFFSET);

/**
* Even though we set TEXT_START_SCALE to HEIGHT_SCALE above, we potentially need
* to re-compute this here since the HEIGHT_SCALE may have changed.
*/
END_SCALE = doTitleAndButtonTextsMatch ? `scale(${WIDTH_SCALE}, ${HEIGHT_SCALE})` : `scale(${HEIGHT_SCALE})`;
}

/**
* When the title and back button texts match
* then they should overlap during the page transition.
* If the texts do not match up then the large title text scale adjusts
* to not perfectly match the back button text otherwise the
* proportions will be incorrect.
* When the texts match we scale both the width and height to account for
* font weight differences between the title and back button.
* The midpoints of the back button and the title should align such that the back
* button and title appear to be centered with each other.
*/
const doTitleAndButtonTextsMatch = backButtonTextEl.textContent?.trim() === largeTitleEl.textContent?.trim();

const WIDTH_SCALE = backButtonTextBox.width / largeTitleTextBox.width;
const HEIGHT_SCALE = backButtonTextBox.height / (largeTitleTextBox.height - LARGE_TITLE_SIZE_OFFSET);

const START_SCALE = 'scale(1)';

const END_SCALE = doTitleAndButtonTextsMatch ? `scale(${WIDTH_SCALE}, ${HEIGHT_SCALE})` : `scale(${HEIGHT_SCALE})`;
const backButtonMidPoint = backButtonBox.top + backButtonBox.height / 2;
const titleMidPoint = (largeTitleBox.height * HEIGHT_SCALE) / 2;
const END_TRANSLATE_Y = `${backButtonMidPoint - titleMidPoint}px`;

const BACKWARDS_KEYFRAMES = [
{ offset: 0, opacity: 0, transform: `translate3d(${END_TRANSLATE_X}, ${END_TRANSLATE_Y}, 0) ${END_SCALE}` },
Expand Down

0 comments on commit bd8d065

Please sign in to comment.