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

Change menu label to Breadcrumb menu #696

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

eerison
Copy link
Contributor

@eerison eerison commented Oct 6, 2022

Adding a name more specific for breadcrumb menu

I am targeting this branch, because This issue is into this branch.

Changelog

### Changed
- Changed `menu label` to `Breadcrumb menu`

It will sove the issue when we depends the block bundle and seo bundle, like in page bundle

Screenshot 2022-10-06 at 10 35 11

@eerison eerison marked this pull request as ready for review October 6, 2022 08:39
@eerison eerison changed the title Change menu label Change menu label to Breadcrumb menu Oct 6, 2022
@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

@VincentLanglet Can I let the others translation missing it?
how do you usually handle new translations for all languages?

@VincentLanglet
Copy link
Member

@VincentLanglet Can I let the others translation missing it? how do you usually handle new translations for all languages?

I generally try to add translation thanks to some knowledge, review or translator.

In french, breadcrumb menu is Menu fil d'Ariane

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

@VincentLanglet Can I let the others translation missing it? how do you usually handle new translations for all languages?

I generally try to add translation thanks to some knowledge, review or translator.

In french, breadcrumb menu is Menu fil d'Ariane

Ok I asked some guys and They suggested to translate Menu only, because Breadcrumb doesn't make sense!

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

Just to have a different icon, what do you think about this one

https://fontawesome.com/v5/icons/angle-double-right?s=solid&f=classic

or should I keep the same (bars)?

@VincentLanglet
Copy link
Member

Just to have a different icon, what do you think about this one

https://fontawesome.com/v5/icons/angle-double-right?s=solid&f=classic

or should I keep the same (bars)?

This icon might be a good idea indeed

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

Screenshot 2022-10-06 at 11 17 30

Ok then it stays like this, I liked it 💅🏼

@VincentLanglet
Copy link
Member

We're changing from fa to fas are we sure font awesome 5 is used by all the seoBundle users ?
Or should we stay with something like <i class="fa fa-angle-double-right"></i> ?

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

We're changing from fa to fas are we sure font awesome 5 is used by all the seoBundle users ? Or should we stay with something like <i class="fa fa-angle-double-right"></i> ?

hmmm well I was following the sonata admin 4!

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

We're changing from fa to fas are we sure font awesome 5 is used by all the seoBundle users ? Or should we stay with something like <i class="fa fa-angle-double-right"></i> ?

hmmm well I was following the sonata admin 4!

I tested and it works on both way fa or fas

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

But as we require block bundle 4, I guess the user is more or less forced to use sonata 4, don't you?

@VincentLanglet
Copy link
Member

But as we require block bundle 4, I guess the user is more or less forced to use sonata 4, don't you?

We dont force to use sonataAdmin 4. People might use just BlockBundle and SeoBubdle maybe...

I dunno where come from this icon, so I dunno if we can use font awesome 5 without a BC break...

@eerison
Copy link
Contributor Author

eerison commented Oct 6, 2022

well Ok lets use fa

@eerison
Copy link
Contributor Author

eerison commented Oct 11, 2022

@jordisala1991 could you review please 👀

@jordisala1991
Copy link
Member

Thank you @eerison

@jordisala1991 jordisala1991 merged commit f5813a7 into sonata-project:3.x Oct 20, 2022
@eerison eerison deleted the Change_menu_label branch October 20, 2022 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants