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

Making max-width responsive #26

Closed
roni-estein opened this issue May 3, 2021 · 8 comments
Closed

Making max-width responsive #26

roni-estein opened this issue May 3, 2021 · 8 comments

Comments

@roni-estein
Copy link
Contributor

Due to the sm:w-full in the modal's body from tailwind ui, you need a way to control the max-width or it will flicker out at smaller breakpoints if your size is too large.

Size is being set here as only one class, but since it's css there is an easy win that doesn't require a huge programmatic change.

https://github.com/livewire-ui/modal/blob/112b521bbd9eef4c188b9eaa0b274dbe424b6578/resources/js/modal.js#L37

This can be modified to instead of just pushing in that one value, I'd use a look up table and pull it from there: here is a working one I tested.

public static array $sizes = [
        'sm' => 'sm:max-w-sm',
        'md' => 'sm:max-w-md',
        'lg' => 'sm:max-w-md md:max-w-lg',
        'xl' => 'sm:max-w-md md:max-w-xl',
        '2xl' => 'sm:max-w-md md:max-w-xl lg:max-w-2xl',
        '3xl' => 'sm:max-w-md md:max-w-xl lg:max-w-3xl',
        '4xl' => 'sm:max-w-md md:max-w-xl lg:max-w-3xl xl:max-w-4xl',
        '5xl' => 'sm:max-w-md md:max-w-xl lg:max-w-3xl xl:max-w-5xl',
        '6xl' => 'sm:max-w-md md:max-w-xl lg:max-w-3xl xl:max-w-5xl 2xl:max-w-6xl',
        '7xl' => 'sm:max-w-md md:max-w-xl lg:max-w-3xl xl:max-w-5xl 2xl:max-w-7xl',
    ]; 

I used it in my own modal system, I was thinking of pulling in this one to solve my child model issue but there are two outstanding tickets that i need solved first. Luckily they both have simple fixes :)

Hope this helps! I'm excited to start using this!

@gcw07
Copy link

gcw07 commented May 3, 2021

This and the current implementation really ties it to Tailwind. I love Tailwind and use it in majority of my projects, but it seems like this shouldn't be so tightly integrated when it doesn't have to be. It is one thing for the default to be Tailwind specific, but it should be able to be used with other CSS frameworks.

The solution seems to be that instead of setting just a size 'sm', 'md', 'lg', etc., that a CSS class just be set. So you could just pass in 'sm:max-w-sm' or 'sm:max-w-md md:max-w-xl lg:max-w-3xl xl:max-w-5xl 2xl:max-w-7xl' and have that class variable merged just like it is now into the template. This would allow this package to be used with other CSS frameworks as the developer would just need to publish the template and change the classes, but then set the size via a style related to that framework.

@roni-estein
Copy link
Contributor Author

Grant, you bring up a valid point, but this issue is narrowly focused on responsive widths. This entire package is packed with tailwind and will completely implode without it. Unless there was an alternative.

What you are suggesting a complete rebuild. To some arbitrary css maybe bootstrap but perhaps with publishable assets. And perhaps a command to choose between some presets. Which sounds great. But it seems like that should be a different issue.

@gcw07
Copy link

gcw07 commented May 3, 2021

Most of the Tailwind code could be more generalized without any rebuild. The most difficult solution would be changing these lines:

https://github.com/livewire-ui/modal/blob/main/resources/js/modal.js#L69
https://github.com/livewire-ui/modal/blob/main/resources/js/modal.js#L71

The rest of Tailwind code is in the blade file that is publishable, so easily changable by anybody and then these

https://github.com/livewire-ui/modal/blob/main/resources/js/modal.js#L30
https://github.com/livewire-ui/modal/blob/main/resources/js/modal.js#L37

Both of those could be switched to a general "maxWidthClass" or something. Why I added it to this thread, is since it was related to changing the max width anyways. But all of this would be up to @PhiloNL anyways. Just an idea to make it easier to use without having to fork the entire project to use it with another CSS framework.

@roni-estein
Copy link
Contributor Author

roni-estein commented May 3, 2021

Ahh I see your point, yes that CSS is simply the fix for the visual issue with the CSS in its current state, but if everything is publishable and configurable, it makes sense that this should be as well. Thats a valid point. It really doesn't matter where it's stored as long as there is a way to set a key and what is put there as a result of the key.

The reason the it's even needed is when the internal items in the modal naturally push the drawing outside of the width of the screen and not the max-width of the modal.

I only exposed the bug because I had a design that required a table in a specific modal and was surprised when people with specific devices mentioned no modal showed up. but the screen overlay, the bg-gray-500 bg-opacity-75.

So this solution proposed is simply to deal with that specific issue that has cropped up for a few people using the tailwind modal as is when we are parameterizing the size.

@PhiloNL
Copy link
Contributor

PhiloNL commented May 5, 2021

Thanks @roni-estein I will take a look at this, somewhere this week 😄 It's taking a bit more time than usual.

@gcw07 @roni-estein I will take a look at generating a scoped Tailwind CSS file that can be used when working with other CSS frameworks. Thanks for the feedback.

@roni-estein
Copy link
Contributor Author

Just had a few seconds between tasks, and I really to delegate my modals to this package, here is a quick illustration of the breakpoint issue. With the default install of livewire and tailwind 2.1+.

Component

<?php

namespace App\Http\Livewire\Modals;

use App\Models\Review;
use LivewireUI\Modal\ModalComponent;

class DeleteReview extends ModalComponent
{
    public ?Review $review;
    
    public function mount(Review $review)
    {
        $this->review = $review;
    }
    
    
    public function delete()
    {
        $this->review->delete();
        
        $this->forceClose()->closeModal();
    }
    
    public function cancel()
    {
        $this->closeModal();
    }
    
    public function render()
    {
        return view('livewire.modals.delete-review');
    }
}

Blade View (literally cut and pasted from tailwindu, I didn't set up events on the buttons yet but just to show the maxwidth issue)

<div class="bg-white px-4 pt-5 pb-4 sm:p-6 sm:pb-4">
    <div class="sm:flex sm:items-start">
      <div
        class="mx-auto flex-shrink-0 flex items-center justify-center h-12 w-12 rounded-full bg-red-100 sm:mx-0 sm:h-10 sm:w-10">
        <!-- Heroicon name: outline/exclamation -->
        <svg class="h-6 w-6 text-red-600" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24"
             stroke="currentColor" aria-hidden="true">
          <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2"
                d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z"/>
        </svg>
      </div>
      <div class="mt-3 text-center sm:mt-0 sm:ml-4 sm:text-left">
        <h3 class="text-lg leading-6 font-medium text-gray-900" id="modal-title">
          Deactivate account
        </h3>
        <div class="mt-2">
          <p class="text-sm text-gray-500">
            Are you sure you want to deactivate your account? All of your data will be permanently removed. This action
            cannot be undone.
          </p>
        </div>
      </div>
    </div>
  </div>
  <div class="bg-gray-50 px-4 py-3 sm:px-6 sm:flex sm:flex-row-reverse">
    <button type="button"
            class="w-full inline-flex justify-center rounded-md border border-transparent shadow-sm px-4 py-2 bg-red-600 text-base font-medium text-white hover:bg-red-700 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-red-500 sm:ml-3 sm:w-auto sm:text-sm">
      Deactivate
    </button>
    <button type="button"
            class="mt-3 w-full inline-flex justify-center rounded-md border border-gray-300 shadow-sm px-4 py-2 bg-white text-base font-medium text-gray-700 hover:bg-gray-50 focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500 sm:mt-0 sm:ml-3 sm:w-auto sm:text-sm">
      Cancel
    </button>
  </div>

This is it in action.

Screen.Recording.2021-05-06.at.12.38.00.PM.mov

@jon-groovy
Copy link

a bit hackey but you can use modalMaxWidth inside your component to "fix" this for now.

Using 5xl from @roni-estein's lookup table as an example, the code below would append the following classes:
sm:max-w-md md:max-w-xl lg:max-w-3xl xl:max-w-5xl

public static function modalMaxWidth(): string
{
    return 'md md:max-w-xl lg:max-w-3xl xl:max-w-5xl';
}

this would obviously break with any update.

@roni-estein
Copy link
Contributor Author

Just wanted to illustrate it, so show what I meant and how prevalent and sneaky it is.

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

No branches or pull requests

4 participants