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

chore: upgrade blade icons library #705

Merged
merged 2 commits into from
May 14, 2021
Merged

Conversation

alfonsobries
Copy link
Member

@alfonsobries alfonsobries commented May 5, 2021

Summary

Related to https://app.clickup.com/t/d11nab

Removes the blade icons library that is no longer used and is not compatible with php8

to make it fully compatible with PHP 8 we still need to update the arkecosystem/crypto lib but the conflicting library is the bitcoin library that is not ready for PHP 8 so we need to wait for a little, apparently, they are already working on it Bit-Wasp/bitcoin-php#879

Update: now upgrades the icons library

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@alfonsobries alfonsobries changed the title feat: remove unused blade icons library chore: remove unused blade icons library May 5, 2021
@alexbarnsley
Copy link
Member

@alfonsobries out of curiosity, why do you believe blade-icons isn't in use anymore? I ask as we use the @svg directive in the <x-ark-icon component

If I remove blade-icons (composer remove blade-ui-kit/blade-icons --ignore-platform-reqs), it then starts to complain about the directive

image

@alexbarnsley alexbarnsley self-assigned this May 11, 2021
@alfonsobries
Copy link
Member Author

@alexbarnsley mmm🤔 remember searching in the project for references to the blade directive without finding any but maybe I missed something, ill check again in a moment

@alexbarnsley
Copy link
Member

Yeh I noticed @svg isn't used in the main codebase but you will find instances of the icon component being used

https://github.com/ArkEcosystem/laravel-ui/blob/master/resources/views/icon.blade.php#L32

@alexbarnsley alexbarnsley marked this pull request as draft May 11, 2021 19:56
@alexbarnsley
Copy link
Member

I've converted the PR to draft for now

@alfonsobries
Copy link
Member Author

@alexbarnsley you are right, the library is actually needed my bad,

Now I'm upgrading the library to the next version that works in PHP8 everything seems to works fine

@alfonsobries alfonsobries marked this pull request as ready for review May 12, 2021 21:45
@alfonsobries alfonsobries changed the title chore: remove unused blade icons library chore: upgrade blade icons library May 12, 2021
@ItsANameToo ItsANameToo merged commit c1c479c into develop May 14, 2021
@ItsANameToo ItsANameToo deleted the chore/remove-blade-icons-lib branch May 14, 2021 08:14
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