forked from symfony/ux
-
-
Notifications
You must be signed in to change notification settings - Fork 0
[Site] Improve UX icon search #13
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
Open
kbond
wants to merge
111
commits into
kbond:site-ux-icons
Choose a base branch
from
smnandre:site-ux-icons
base: site-ux-icons
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(remove remaining Node16 deprecations)
(avoid running it 40x a day when no doc files have been changed)
…weaverryan) This PR was merged into the 2.x branch. Discussion ---------- [Site] Using URL binding on live components demo | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | None | License | MIT Easy win - on https://ux.symfony.com/live-component, as you search in the demo, the URL will now update with the search query. Thanks to that great new feature in 2.14 🔥 Cheers! Commits ------- dfe2ceb [Site] Using URL binding on live components demo
kbond
commented
Feb 6, 2024
ux.symfony.com/src/Iconify.php
Outdated
| $item->expiresAfter(604800); // 1 week | ||
|
|
||
| return $this->http->request('GET', 'https://api.iconify.design/collections') | ||
| $this->http->request('GET', 'https://api.iconify.design/collections') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
| $this->http->request('GET', 'https://api.iconify.design/collections') | |
| return $this->http->request('GET', 'https://api.iconify.design/collections') |
|
This is looking really nice, I'm so excited! |
|
Couple comments:
|
… varying structures
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Twig] Removing duplication in docs | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | None | License | MIT Somehow, I think during some rewriting I did, I didn't remove old sections. This removes the duplicated sections, but keeps one piece that was new. Cheers! Commits ------- 2dcd7b3 [Twig] Removing duplication in docs
This PR was merged into the 2.x branch. Discussion ---------- [Turbo] Allowing version 8 | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Issues | None | License | MIT Turbo 8 is now beta! This allows Turbo 8 to be installed. There are no BC breaks that would affect us that i'm aware of, which is not surprising as we do very little with Turbo other than supply it to the user. Once we merge and release this, users installing TurboBundle will get v8 by default, even while in beta stage. From AssetMapper's perspective, that happens because https://data.jsdelivr.com/v1/packages/npm/`@hotwired`/turbo/resolved already selects beta. I can't find any specifier - e.g. https://data.jsdelivr.com/v1/packages/npm/`@hotwired`/turbo/resolved?specifier=stable that selects the latest, non-beta version. Cheers! Commits ------- 7fe845b Allowing version 8 of Turbo
…rt projects with varied structures using Composer with custom directory structure (yobrx)
This PR was merged into the 2.x branch.
Discussion
----------
[StimulusBundle] UxPackageReader class doesn't support projects with varied structures using Composer with custom directory structure
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Issues | no
| License | MIT
Hello,
I've encountered an issue with the `UxPackageReader` class within the Symfony UX package, where it fails to accurately detect package existence in projects that have a structure differing from the standard Symfony application structure, despite these projects using Composer for package management.
## Description of the Issue
The `UxPackageReader` class appears to be designed with a certain project structure, which works well for typical Symfony applications. However, in projects that deviate from this structure (for instance, microservices or applications with a modular architecture), the class does not effectively locate and read the Composer-managed packages.
## Specific Scenario
To give a concrete example, my project consists of multiple directories at the root, each representing a sub-project within a larger ecosystem. These sub-projects are designed to have their own `vendor` directory for autoloading purposes. However, all Composer packages are installed in a shared `vendor` directory at the root of the project. This setup is beneficial for my project's architecture but unfortunately, `UxPackageReader` is unable to recognize the packages in this unique structure.
### Example :
```
app1
|- vendor
|- composer
|- autoload-*.php -> reference files to ../vendor/
|- ... and other files of composer
app2
|- subapp1
|- vendor
|- composer
|- autoload-*.php -> reference files to ../../vendor/
|- ... and other files of composer
|- subapp2_legacy
|- vendor
|- composer
|- autoload-*.php -> reference files to ../../vendor/
|- ... and other files of composer
vendor
|- symfony
|- packages
|- v6.3
|- _files of package_
|- v6.4
```
## Proposed Solution
To address this limitation, I have implemented a workaround by utilizing the `InstalledVersions` class from Composer, whenever it is available. This class offers a more robust and flexible way to check for the existence of packages, as it does not rely on the project's directory structure but instead directly reads from Composer's autoload mechanisms and installed packages metadata.
## Suggested Changes
I suggest that the Symfony UX team considers integrating a similar approach into the `UxPackageReader` class. By leveraging `InstalledVersions` or an equivalent method, Symfony UX can enhance its compatibility with a broader range of project structures that use Composer, thus increasing its versatility and usability.
This modification would not only resolve the issue for projects with unique structures but also align the package detection mechanism with Composer's native capabilities, ensuring a more consistent and reliable detection process.
My solution is not testable because it uses Composer class with static methods. Therefore, I was unable to write tests for it. However, I am open to any suggestions for improvement.
Commits
-------
0b17512 Used a Composer class to check for package existence in projects with varying structures
This PR was merged into the 2.x branch. Discussion ---------- CS Update | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | n/a | License | MIT Updates to the latest Symfony CS rules. Refs: PHP-CS-Fixer/PHP-CS-Fixer#7773 & symfony/symfony#53612 Commits ------- aad4e17 minor: update cs
…(weaverryan) This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Turbo] Fixing support for not using old ClassUtils | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Issues | Fix symfony#1469 | License | MIT Commits ------- 4753fc3 [Turbo] Fixing support for not using old ClassUtils
…re temporarily (weaverryan) This PR was merged into the 2.x branch. Discussion ---------- Bumping 2.14.2 CHANGELOG and reverting minor feature temporarily | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | None | License | MIT Hi! I'd like to get a bug fix release out - especially for the UX turbo package and newer Doctrine versions. Out of an abundance of caution, I'm reverting the Turbo 8 support in this PR, so the patch release is clean. After merging, I'll remake a PR to re-add the support. Cheers! Commits ------- 3eb560a Bumping 2.14.2 CHANGELOG and reverting minor feature temporarily
This PR was merged into the 2.x branch. Discussion ---------- [Turbo] add missing use statement | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Issues | | License | MIT The new `Ux\...\ClassUtil` and the legacy `Doctrine\...\ClassUtils` are very similar. Adding the missing use statement with an alias to avoid IDE confusion :) Commits ------- 6e5fe19 [Turbo] add missing use statement
…m (kbond) This PR was merged into the 2.x branch. Discussion ---------- [Twig] Add (alternate) attribute rendering system | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Issues | n/a | License | MIT This is an alternate to symfony#1404 as suggested by Ryan [here](symfony#1404 (comment)). This PR allows the following: ```twig <div class="{{ attributes.render('class') }} appended-default" style="prepended-default {{ attributes.render('style') }}" data-foo="{{ attributes.render('data-foo')|default('replaced-default') }}" {{ attributes }} > ``` When calling `attributes.render('name')` ~(or magically via `attributes.name`)~, the attribute is marked as _rendered_. Later when calling just `{{ attributes }}`, the attributes marked as already rendered are excluded. Whether or not an attribute is considered rendered is only evaluated when converting `ComponentAttributes` to a string. TODO: - [x] Docs - [x] Add test ensuring works in real twig component Commits ------- 0d027d5 feat(twig): Add attribute rendering system
…e autocomplete fields (jakubtobiasz) This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Autocomplete] Allow passing extra options to the autocomplete fields | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Issues | n/a | License | MIT This PR still needs some tweaks and tests, but I want to validate my idea. Currently, if we pass anything as a third argument in the `->add()` method, it is only used on the form render, and is fully ignored during the AJAX call.  While implementing UX's Autocomplete in Sylius, I wanted to complete the following user story ``` Given I want to edit a taxon When I want to assign a parent taxon to it And I check a list of available taxa Then I should see all taxa except the edited one ``` So basically, I have to pass a **current** taxon's ID to the autocomplete's query. As we know, currently it's not possible. So after contacting `@weaverryan`, I decided to implement a mechanism similar to the one from Live Components. When you pass an array of options as a `3rd` argument, you can use a special `extra_options` key, which is an array consisting `scalars`/`arrays`/`nulls`. Next, when the form is rendered, I get these values, calculate a checksum for them, then pass them through `json_encode` and `base64_encode` functions. In the end, I glue them to the `url` values in the `\Symfony\UX\Autocomplete\Form\AutocompleteChoiceTypeExtension::finishView` method. So, basically with the following configuration:  we end up with the following HTML code:  I decided to "glue" the `extra_options` to the URL, as I didn't have to deal with JS. Of course, I do not exclude a chance to refactor it, as a whole method should be refactored anyway. Finally, the controller decodes the data, checks the checksum and passes the values to the autocomplete's form. Commits ------- 308daef [Autocomplete] Allow passing extra options to the autocomplete fields
…luz) This PR was merged into the 2.x branch. Discussion ---------- [Doc] Fix a minor syntax issue in docs | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | Issues | - | License | MIT Commits ------- 442e262 Update index.rst
05f5608 to
34356db
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.