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

Orientation media query #135

Open
marcianosr opened this issue May 27, 2020 · 7 comments
Open

Orientation media query #135

marcianosr opened this issue May 27, 2020 · 7 comments
Labels
question Further information is requested

Comments

@marcianosr
Copy link

Hi,

I'm using this package and I stumbled upon weird behaviour defining interactions. I took over the docs example:

export const interactions: { [key in Interaction]: string } = {
  landscape: "(orientation: landscape)",
  portrait: "(orientation: portrait)",
};

Assuming that this would work like this:

<Media interaction="portrait"> 
    <p>portrait view</p>
</Media>
<Media interaction="landscape"> 
    <p>landscape view</p>
</Media>

However, it is working the other way round which had me switch the interactions config up:

export const interactions: { [key in Interaction]: string } = {
  landscape: "(orientation: portrait)",
  portrait: "(orientation: landscape)",
};

It seems that, looking at the docs, this is not intentional and a bug.

@alloy
Copy link
Collaborator

alloy commented May 28, 2020

Yikes, thanks for reporting! There’s a lot of negation that happens when actually defining these media rules, so that might be where it’s going wrong.

@damassi let me know if you need a second pair of eyes on fixing this.

@fvpDev
Copy link

fvpDev commented Jun 3, 2020

Waiting for fix in order to implement this :)

@zephraph
Copy link
Contributor

zephraph commented Jun 5, 2020

Thanks for reporting! I'm looking into this and will report back when I have updates or a fix. 👍

@zephraph
Copy link
Contributor

zephraph commented Jul 25, 2020

Hey @marcianosr sorry about the long delay, it's been a busy few months. Could you possibly provide a reproduction that shows this issue?

I've got a minimal codesandbox example here but according to what I'm seeing it's functioning correctly. Visit this link to test it on a device: https://7ho2t.csb.app/.

Got any more details? Perhaps what device/browser your testing it on?

@zephraph zephraph added the question Further information is requested label Jul 25, 2020
@mfanuzzi
Copy link

Hey @zephraph! My team just ran into this one as well, and I think I can help point you towards an answer.

In the codesandbox example you posted, you can see that, in addition to modifying the DOM, the CSS classes fresnel-interaction-landscape and fresnel-interaction-portrait are being applied (correctly.) But those classes are not defined.

What we noticed, though, is that in our case, those CSS classes had rules opposite to their intentions. Here's an example of what we saw:

fresnel interaction

You can see there that the right content is actually being loaded, but unfortunately the CSS class applied to it hides the contents.

This led me down a rabbit hole of looking into how those classes were generated. The logic here is quite complex so I'm not going to try to PR this, but here's what worked for me:

On line 88 of Interactions.js (from the module dist, or 52 of Interactions.ts) there is the following logic:

  public shouldRenderMediaQuery(
    interaction: string,
    onlyMatch: string[]
  ): boolean {
    return !!(onlyMatch && onlyMatch.includes(interaction))
  }

I feel that this is where the error lies. I changed that logic to:

    return !!(onlyMatch && onlyMatch.includes(interaction)) == false

...and voila, the DOM action + CSS started matching.

Admittedly there are better ways to write this, and I can't dive into the logic you've got behind this. But I hope this helps y'all reproduce the issue and find a fix.

@zephraph
Copy link
Contributor

Thanks for the sleuthing! I'll dig back into it and see what I come up with.

@tommy92
Copy link

tommy92 commented Jul 6, 2021

I can see the same issue, so it still occurs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants