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.

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

bug: Collapsible Large Titles Flicker #27060

Closed
3 tasks done
seanaguinaga opened this issue Mar 30, 2023 · 16 comments · Fixed by #28277
Closed
3 tasks done

bug: Collapsible Large Titles Flicker #27060

seanaguinaga opened this issue Mar 30, 2023 · 16 comments · Fixed by #28277
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@seanaguinaga
Copy link

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

With version 7, the collapsed title flashes upon navigation

Does not happen with version 6

Screen.Recording.2023-03-29.at.5.01.40.PM.mov

Please see video ^

Expected Behavior

Would not flash, as in version 6

Steps to Reproduce

upgrade to version 7

see flash on collapsed title

Code Reproduction URL

No response

Ionic Info

Ionic:

   Ionic CLI : 6.20.8

Utility:

   cordova-res : not installed globally
   native-run  : 1.7.2

System:

   NodeJS : v18.15.0
   npm    : 9.5.0
   OS     : macOS

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Mar 30, 2023
@liamdebeasi liamdebeasi added the ionitron: needs reproduction a code reproduction is needed from the issue author label Mar 30, 2023
@ionitron-bot
Copy link

ionitron-bot bot commented Mar 30, 2023

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@ionitron-bot ionitron-bot bot removed the triage label Mar 30, 2023
@seanaguinaga
Copy link
Author

hey @liamdebeasi how do I give you access to a private monorepo?

@liamdebeasi
Copy link
Contributor

I recommend reproducing this in an Ionic starter application and providing a link to the repo. This way you can have a minimal reproduction without providing any secret/private code.

@seanaguinaga
Copy link
Author

seanaguinaga commented Mar 30, 2023

https://github.com/seanaguinaga/large-title-repro

A little different - duration is longer - but I'm certain it's the same cause

Thank you for all that you do Liam (and team) - Ionic is amazing (as you know)

@averyjohnston averyjohnston added triage and removed ionitron: needs reproduction a code reproduction is needed from the issue author labels Mar 31, 2023
@seanaguinaga
Copy link
Author

Also happens here on mlynch's repo on first render

https://github.com/mlynch/nextjs-tailwind-ionic-capacitor-starter

@milanharia
Copy link

@liamdebeasi @amandaejohnston I have managed to recreate this with an ionic starter project, it is clearly visible when you make the content a different colour other than white.

This is the code for my page:

<IonPage>
      <IonHeader>
        <IonToolbar color="primary">
          <IonTitle>Tab 1</IonTitle>
        </IonToolbar>
      </IonHeader>
      <IonContent fullscreen color="primary">
        <IonHeader collapse="condense">
          <IonToolbar color="primary">
            <IonTitle size="large">Tab 1</IonTitle>
          </IonToolbar>
        </IonHeader>
      </IonContent>
    </IonPage>
Simulator.Screen.Recording.-.iPhone.14.-.2023-04-11.at.10.49.27.mp4

@thetaPC
Copy link
Contributor

thetaPC commented May 19, 2023

@seanaguinaga please provide a video clip of the flickering that you are experiencing on your repo, large-title-repro. This would allow us to track the bug easier since I have not been able to replicate it on your repo.

@thetaPC
Copy link
Contributor

thetaPC commented May 19, 2023

@milanharia please provide the reproduction that you mention. I wasn't able to replicate it using the React Tabs starter and this is a tricky bug.

@thetaPC thetaPC added the needs: reply the issue needs a response from the user label May 19, 2023
@ionitron-bot ionitron-bot bot removed the triage label May 19, 2023
@seanaguinaga
Copy link
Author

Title comes in showing - note the name of the person on the navigated to page

Title.Comes.in.Showing.mov

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels May 22, 2023
@seanaguinaga
Copy link
Author

Here's the fast flicker on refresh of that detail page

FLicker.on.Refresh.mov

@thetaPC
Copy link
Contributor

thetaPC commented May 22, 2023

Thank you! I was able to replicate the issue by building the project to iOS.

@thetaPC thetaPC added the type: bug a confirmed bug report label May 22, 2023
@ionitron-bot ionitron-bot bot removed the triage label May 22, 2023
@liamdebeasi liamdebeasi added the package: core @ionic/core package label Jun 13, 2023
@liamdebeasi
Copy link
Contributor

Hi everyone,

Here is a dev build with a proposed fix if anyone is interested in testing: 7.4.3-dev.11696365694.156f41d3

Install Example:

npm install @ionic/[email protected] @ionic/[email protected]

github-merge-queue bot pushed a commit that referenced this issue Oct 5, 2023
…load (#28277)

Issue number: resolves #27060

---------

<!-- 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 main header is controlled by the header with `collapse="condense"`
set:
https://github.com/ionic-team/ionic-framework/blob/a04a11be3571faa99c751edc034462e94a977e95/core/src/components/header/header.tsx#L144

The collapse header will hide the main header and then show it once the
user has scrolled enough. However, if the main header is rendered before
the collapse header is rendered, then the main header will be visible
for a brief moment before being hidden by the collapse header. This
gives the perception of flicker that is reported on the linked issue.

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

- The main header will be hidden on load if it loads before the collapse
header

The selector was written in a way such that once the collapse header
loads, this CSS no longer applies (since the collapse header will add
`.header-collapse-main` to the main header)

| `main` | branch |
| - | - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/3cb11a57-e084-435a-89c2-e1c2afba04b1"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/c5caeb5e-3b33-4598-986f-bf097c46251c"></video>
|

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

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

Note: `:has` browser compat is still fairly new. However, it is
available on both Chromium and WebKit browsers (and has been for at
least a year): https://caniuse.com/?search=%3Ahas

Given that this bug is a fairly minor UI glitch (as opposed to something
that would cause an app to crash or otherwise malfunction), I think this
is an acceptable tradeoff. As time goes on this will become less of a
concern as more users update their devices.

Dev build: `7.4.3-dev.11696365694.156f41d3`
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #28277, and a fix will be available in an upcoming release of Ionic Framework. Feel free to keep testing the dev build, and let me know if you run into any issues.

@moerphie
Copy link

@liamdebeasi #28277 hasn't fixed it for me.

<template>
    <ion-page ref="xxxpage">
        <ion-header :translucent="true">
            <ion-toolbar>
                <ion-title>XXX</ion-title>
            </ion-toolbar>
        </ion-header>

        <ion-content :fullscreen="true">
            <ion-header collapse="condense">
                <ion-toolbar>
                    <ion-title size="large">
                        XXX
                    </ion-title>
                </ion-toolbar>
            </ion-header>
        </ion-content>
    </ion-page>
</template>
"@ionic/vue": "7.5.0",
"@ionic/vue-router": "7.5.0",
ionic.mov

@liamdebeasi
Copy link
Contributor

Please file a new issue with a reproduction case.

github-merge-queue bot pushed a commit that referenced this issue Nov 9, 2023
…rop not reflected (#28472)

Issue number: resolves #28466

---------

<!-- 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. -->

In #27060 I fixed an
issue where the main title would be visible briefly before the
collapsible large title a) was configured and b) hid the main title. I
accomplished this by using CSS to target
`ion-header[collapse="condense"]`. However, I failed to account for when
the property is not reflected on the host. Some JS frameworks allow the
property to remain on the element but some do not.

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

- I improved this fix by also targeting the class set on the host. This
class is set regardless of property reflection status.

| main | branch |
| - | - |
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/991523da-8549-451b-930f-5df45c2783de"></video>
| <video
src="https://github.com/ionic-team/ionic-framework/assets/2721089/149c9546-2d9b-42a2-89f1-a17fa146aee6"></video>
|

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## 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.5.4-dev.11699282935.1db450b0`

---------

Co-authored-by: Brandy Carney <[email protected]>
Copy link

ionitron-bot bot commented Nov 15, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Nov 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants