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

Dark Mode v2 #1229

Closed
wants to merge 3 commits into from
Closed

Dark Mode v2 #1229

wants to merge 3 commits into from

Conversation

fotrino
Copy link
Contributor

@fotrino fotrino commented Jan 19, 2023

This PR brings back Dark Mode changes introduced in PR 1209 without causing issues to existing applications with Jetstream installed as it was mentioned in Issue 1226.

Changes

  • As suggested by @jessarcher in this comment, the views from views/resources remain unchanged to avoid issues with previous installs.

  • Original files from views/resources have been copied to stubs/livewire/resources/views/ to keep track of changes.

  • On new installations, the following directories stubs/livewire/resources/views/components stubs/livewire/resources/views/mail will be copied to views/vendor/jetstream/.

  • On new installations without --dark directive, the command php artisan vendor:publish --tag=jetstream-views --force was removed. Views are already published as mentioned in point above.

  • The changes introduced to strip dark classes PR 1224, dark nav menu in PR 1225 have been included.

Step to verify Issue 1226 is Fixed

  1. Install new laravel app
  2. Install Jetstream 2.16 composer require laravel/jetstream
  3. Run php artisan jetstream:install livewire --teams
  4. Check that there are not any published jetstream views in views/vendor/jetstream
  5. Update Jetstream to "laravel/jetstream": "dev-dark-mode-v2"
  6. Include the PR repo.
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/fotrino/jetstream.git"
        }
      ],
  1. Run composer update

Update Install CMD to copy components from livewire stubs instead of publishing views
@jessarcher
Copy link
Member

Thanks for tackling this one again @fotrino! I'll check it out properly once it's marked ready.

@fotrino fotrino marked this pull request as ready for review January 23, 2023 23:32
@fotrino
Copy link
Contributor Author

fotrino commented Jan 23, 2023

Hi @jessarcher when you have a chance, take a look at the PR. Thanks!

@taylorotwell
Copy link
Member

So does this essentially duplicate the Livewire views in Jetstream for the time being? The old legacy views and the new views that support dark mode?

@fotrino
Copy link
Contributor Author

fotrino commented Jan 25, 2023

Yes, exactly. This will prevent any issues in previous installations without published views & new ones will work as user defines upon installation (with or without dark mode) because all the components are being published from the livewire stubs.

@taylorotwell
Copy link
Member

@fotrino ... @jessarcher can correct me if I'm wrong, but we also batted around the idea of just tagging a Jetstream 3.0 - this would allow us to make the change without having to have the duplicated views for backwards compatibility.

@fotrino
Copy link
Contributor Author

fotrino commented Jan 27, 2023

@taylorotwell that sounds even better!

What do you think about moving the files from resources/views/ to stubs/livewire/resources/views/?
I think it would make it easier to determine that those components belongs to the livewire stubs.

Also, the following command php artisan vendor:publish --tag=jetstream-views from JetstreamServiceProvider could be removed, since the files would already be published in new installations & previous ones upgrading from 2.x to 3.x should have already publish their assets in order to upgrade.

@jessarcher
Copy link
Member

I agree re: moving this to Jetstream 3.0. We'll just need to mention in the upgrade guide about publishing views prior to upgrading (if the user hasn't already).

What do you think about moving the files from resources/views/ to stubs/livewire/resources/views/? I think it would make it easier to determine that those components belongs to the livewire stubs.

I had already done some work on this in #1118. If you take a look at master you'll notice the root-level resources directory is already gone.

Also, the following command php artisan vendor:publish --tag=jetstream-views from JetstreamServiceProvider could be removed, since the files would already be published in new installations & previous ones upgrading from 2.x to 3.x should have already publish their assets in order to upgrade.

I'd already removed this in master as well 🙌

I've just gone ahead and merged the latest 2.x changes into master and fixed all of the conflicts, so it should hopefully be straight-forward to apply these dark mode changes there 🤞

The main conflict you may run into with the Inertia stack is that in 3.x the authenticated user is now shared under auth.user instead of just user (#1073).

Thanks again for your work on this!

@taylorotwell
Copy link
Member

taylorotwell commented Jan 27, 2023

Hey @fotrino! Based on @jessarcher's feedback, could you send this PR to 10.x accordingly? Thanks!

@fotrino fotrino mentioned this pull request Jan 28, 2023
@fotrino
Copy link
Contributor Author

fotrino commented Jan 28, 2023

Hey @taylorotwell, just created a PR on master based on the feedback from @jessarcher 😃

@fotrino fotrino deleted the dark-mode-v2 branch January 28, 2023 15:10
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

Successfully merging this pull request may close these issues.

3 participants