AlaskaAirlines / auro-flightline

A custom element that represents the intermediate details between a departure and arrival station on a flight.
https://auro.alaskaair.com/components/auro/flightline
Apache License 2.0
0 stars 2 forks source link

auro-flightline: Distorted Flight Line #55

Closed ggrewal108 closed 1 year ago

ggrewal108 commented 1 year ago

Describe the bug

The flight changes team is working on sorting sorting flight options within our react microsite and we occasionally see the flight line either disappear - whenever the flight has one segment - or see it distorted - when there are more than one segments.

To Reproduce

We have not noticed this behavior on initial load of our page, however, when we try to sort the flight options displayed on the screen, we see this behavior. When the sorting is triggered within our react app, the flight results are sorted and dispatched to the state again which causes a re-rendering of the options on the page.

Expected behavior

Once the results are sorted, we expected the flight line to be rendered as usual without any distortion.

Screenshots

Screenshot 2023-07-12 at 1 01 52 PM Screenshot 2023-07-12 at 1 02 03 PM

Desktop (please complete the following information):

Smartphone (please complete the following information):

Did not directly test the issue on a smartphone, however tried responsive design mode under the develop menu on safari and the issue was seen again.

Additional context

N/A

blackfalcon commented 1 year ago

Can you provide us with a reproducible test environment to troubleshoot this? As of right now, we have no way of reproducing this behavior.

fajar-apri-alaska commented 1 year ago

Hi Team. I can somehow reproduce the first 'distorted' issue. The structure code is something like this:

<div style="display:flex">
  <auro-flightline style="width:100%">
    // Empty segment
  </auro-flightline>

  <auro-flightline style="width:100%">
    // Have one or more segment
    <auro-flight-segment></auro-flight-segment>
  </auro-flightline>
</div>

Preview:

image

I did found out the problem, it seems that the top value from style-flight-segment.scss

// places the connecting line with a pseudo class
.wrapper::before,
.wrapper::after {
  border-top: var(--auro-size-xxxs) solid var(--auro-color-brand-atlas-500);
  position: absolute;
  top: 0.5rem; <-- This value
  z-index: 1;
}

and from style-flightline.scss

// places the connecting line with a pseudo class
.nonstop {
  &::before {
    content: "";
    height: var(--auro-size-xxxs);
    width: 100%;
    position: relative;
    top: 10px; <-- This value
  }
}

are different.

So I tried to make both have the same value and it just fixes, either to make it both 10px or 0.5rem. Preview:

image
fajar-apri-alaska commented 1 year ago

For the the one that are disappearing, I assume that when we put an empty flightline component (without the segment) inside flex-based div, it needs a width: 100% so that it renders.

This one does render:

<div style="flex">
  <span> Left text </span>
  <auro-flightline style={{width: '100%'}}></auro-flightline>
  <span> Right text </span>
</div>
image

This one doesn't:

<div style="flex">
  <span> Left text </span>
  <auro-flightline></auro-flightline>
  <span> Right text </span>
</div>
image
blackfalcon commented 1 year ago

I am still unable to repro this issue? Givin the initial description, I cannot repo this issue with a very similar example on the Auro docsite https://auro.alaskaair.com/components/auro/flight#:~:text=auro%2Dflight%3E-,Mainline,-%2B%20mainline%20connection%20w

@fajar-apri-alaska I am unable to repro your version either. Given your hypothesis, why is this not appearing on the Flight demo page?

fajar-apri-alaska commented 1 year ago

I think the auro-flight demo page / instruction should have no problem.

While I don't know how the devs implement the auro-flight/auro-flightline component, I did found the issue occurs when the code is implemented outside of the given instructions (for example, wrapping the auro-flightline elements with a div according to what I found).

Things that I got for the 'distorted' issue to be reproduced like above example:

  1. There are multiple auro-flightline elements,
  2. Those auro-flightline elements are somehow wrapped inside a container that have display: flex, and
  3. Those auro-flightline elements needs to have width:100% (or else they will disappear as per the 'disappearing' issue).
    <auro-flight flights='["AS 1436"]'>
    <div style="display: flex">
    <auro-flightline style="width: 100%"></auro-flightline>
    <auro-flightline style="width: 100%">
      <auro-flight-segment></auro-flight-segment>
    </auro-flightline>
    </div>
    </auro-flight>

Since I'm going outside of the given instructions, please advise if I should not go into / just ignore this approach.

blackfalcon commented 1 year ago

We're able to troubleshoot this issue and discovered what the root cause of the issue is.

Screen Shot 2023-07-26 at 10 29 04 AM

As seen in the screenshot. when there is a nonstop property on the node that contains a layover, this causes the UI to break. The nonstop property is ONLY to be used when there are no layovers listed for the flight.

@ggrewal108 will look into the data for this scenario with the intent to understand why flights in her list that clearly have layovers are also being tagged as nonstop flights.

The code for nonstop and layovers were never engineered to account for both being used at the same time.

https://github.com/AlaskaAirlines/auro-flightline/blob/63ed29d5214f48ee8de092b2c5d088ab98f969ba/src/style-flightline.scss#L22-L40

ggrewal108 commented 1 year ago

We're able to troubleshoot this issue and discovered what the root cause of the issue is.

Screen Shot 2023-07-26 at 10 29 04 AM

As seen in the screenshot. when there is a nonstop property on the node that contains a layover, this causes the UI to break. The nonstop property is ONLY to be used when there are no layovers listed for the flight.

@ggrewal108 will look into the data for this scenario with the intent to understand why flights in her list that clearly have layovers are also being tagged as nonstop flights.

The code for nonstop and layovers were never engineered to account for both being used at the same time.

https://github.com/AlaskaAirlines/auro-flightline/blob/63ed29d5214f48ee8de092b2c5d088ab98f969ba/src/style-flightline.scss#L22-L40

@blackfalcon Thanks for taking the time to meet me and to go over this issue today. Given the complexity of the issue, I believe it’s going to take me more than a cursory glance to understand what going on underneath the hood and to give this issue the consideration that it deserves. However, at the moment there are other items that require my attention and consideration, therefore I am going to put this on hold and will try to address it at the earliest opportunity possible. Once again, than you for your understanding and your collaboration.

blackfalcon commented 1 year ago

@ggrewal108 no worries. I am curious why this nonstop CSS selector is being added to the DOM at render. The code for how this works is all based on children within the node.

https://github.com/AlaskaAirlines/auro-flightline/blob/63ed29d5214f48ee8de092b2c5d088ab98f969ba/src/auro-flightline.js#L98-L105

Basically, if there are no children in that slot, then you get the nonstop selector added. If there is 1, then nothing is added, if there is > 1 then you get the multiple selector.

By adding things to the DOM, I can't get this to reproduce. There has to be an answer in the data that is rendering the UI on your end.

ggrewal108 commented 1 year ago

@ggrewal108 no worries. I am curious why this nonstop CSS selector is being added to the DOM at render. The code for how this works is all based on children within the node.

https://github.com/AlaskaAirlines/auro-flightline/blob/63ed29d5214f48ee8de092b2c5d088ab98f969ba/src/auro-flightline.js#L98-L105

Basically, if there are no children in that slot, then you get the nonstop selector added. If there is 1, then nothing is added, if there is > 1 then you get the multiple selector.

By adding things to the DOM, I can't get this to reproduce. There has to be an answer in the data that is rendering the UI on your end.

Screenshot 2023-07-28 at 2 03 21 PM

This array is the data that is supplied to the component that renders the flight line and the flight segments. The flight line is always set to render, however, the segments only render when the array that is received has length > 1. In the case where the array is empty, the component is never rendered. When the length is > 1, the destination property is used for iata. I have noticed that when the flight option is initially nonstop but then sorted and the new sorted option has flight segments, thats when the distortion happens. However, the data supplied still seems to be right.

Screenshot 2023-07-28 at 3 02 24 PM Screenshot 2023-07-28 at 3 03 27 PM

Could this possibly be a react problem? I don't believe the problem is necessarily with the data that is supplied to the components.

Screenshot 2023-07-28 at 3 33 18 PM Screenshot 2023-07-28 at 3 32 53 PM
blackfalcon commented 1 year ago

@ggrewal108 this all makes sense.

flight option is initially nonstop but then sorted and the new sorted option has flight segments

That's the smoking gun right there. The nonstop selector is being added because at render the segment is >1. But what you are saying is that post resort the segments are >1, the component is NOT rendering, and that nonstop selector is still there.

Why is the segment being added AFTER the resort?

tjgrist commented 1 year ago

We ended up adding a React key to each auro-flightline element, and that resolved the issue.

Screenshot 2023-08-08 at 9 56 30 AM
blackfalcon commented 1 year ago

@tjgrist can you provide a reason why this is happening and/or why someone has to use this with this component?

I'd like to get this added to the install docs as a reference for future users of the element. This appears to be a React'ism as we have not run into this with the Svelte teams.

ggrewal108 commented 1 year ago

@tjgrist can you provide a reason why this is happening and/or why someone has to use this with this component?

I'd like to get this added to the install docs as a reference for future users of the element. This appears to be a React'ism as we have not run into this with the Svelte teams.

MicrosoftTeams-image

React needs a key to be able to differentiate among all the rendered auro-flightlines or more generically when you render multiples of the same component. It uses that key to determine which component/element needs to be rendered and how much of it needs to be rendered. Since we weren't providing the flight line with the key, it didn't seem to be rendering it from scratch again, therefore, the css in the shadow root wasn't getting updated correctly - It was only updating the stuff it needed to update which were the other things in that component.

tjgrist commented 1 year ago

Literally couldn't have said it better myself

blackfalcon commented 1 year ago

See commit