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

feat: switch to condensed view based on container width #23

Closed geoffrich closed 2 years ago

geoffrich commented 3 years ago

Alaska Airlines Pull Request

Fixes https://github.com/AlaskaAirlines/auro-flight/issues/19

Summary:

Dale suggested looking into the cqfill package to polyfill CSS container queries. However, I was unable to get that package to work with web components (see discussion in linked auro-flight issue). I couldn't find any other stable polyfill, options, so I opted to keep my original ResizeObserver solution instead. This should be relatively easy to swap out for container queries when they're available in browsers -- instead of setting a condensed class, use a media query instead and remove the observer code.

If anyone has suggestions for alternate solutions for me to investigate, let me know.

Before (675px screen width): image

After: image

As a demonstration, add the following HTML to the demo and resize the screen.

<style>
  .grid {
    display: grid;
    grid-template-columns: 1fr 1fr;
    gap: 2rem;
  }
</style>

<div class="grid">
  <auro-flightline id="flight1">
    <auro-flight-segment stopover iata="YAK"></auro-flight-segment>
    <auro-flight-segment stopover iata="WRG"></auro-flight-segment>
    <auro-flight-segment iata="SEA" duration="0h 40m"></auro-flight-segment>
    <auro-flight-segment iata="BOS" duration="1h 40m"></auro-flight-segment>
    <auro-flight-segment iata="DUB" duration="13h 40m"></auro-flight-segment>
  </auro-flightline>
  <auro-flightline id="flight2">
    <auro-flight-segment stopover iata="YAK"></auro-flight-segment>
    <auro-flight-segment stopover iata="WRG"></auro-flight-segment>
    <auro-flight-segment iata="SEA" duration="0h 40m"></auro-flight-segment>
    <auro-flight-segment iata="BOS" duration="1h 40m"></auro-flight-segment>
    <auro-flight-segment iata="DUB" duration="13h 40m"></auro-flight-segment>
  </auro-flightline>
</div>

Type of change:

Please delete options that are not relevant.

Checklist:

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!
-- Auro Design System Team

blackfalcon commented 3 years ago

Given this draft PR, how do you feel about working with this polyfill that is based on the emerging spec?

https://github.com/AlaskaAirlines/auro-flight/issues/19#issuecomment-932537409

geoffrich commented 3 years ago

I'm finally getting back to this today. I'm going to take a look at using the CQ polyfill.

geoffrich commented 3 years ago

Talked to @blackfalcon on a call. I'll need to make the following changes:

geoffrich commented 2 years ago

@blackfalcon finally got back to this. I think I addressed all your comments -- please take another look. Going mobile-first made things a lot cleaner.

Also, I think you linked the wrong issue to this PR -- this addresses https://github.com/AlaskaAirlines/auro-flight/issues/19. The canceled flightline will be addressed separately.

geoffrich commented 2 years ago

Refactored according to suggested name changes. See my comments above. Also updated the tests to ignore the "ResizeObserver limit exceeded" error, which only happened in the tests. Previously, I had wrapped the call in requestAnimationFrame to fix this, but this was inexplicable causing several tests to be skipped. I removed the requestAnimationFrame and told Mocha to treat the error as a warning instead.

@blackfalcon let me know what you think of the name changes (though I know you're off for the holidays soon, so no rush)

blackfalcon commented 2 years ago

I am approving this work given the scope is directly tied to this element's use. Abstracting this into an individual utility will be discussed in the future with a different proposal of work.

Given the current maturity of the @content selector and its current availability behind a chrome flag, it's safe to say that by the time we get to the work needed to make this a shareable utility the official spec will be in the wild.