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

Use Lucide icons in the back end #7278

Merged
merged 11 commits into from
Jun 19, 2024
Merged

Conversation

leofeyer
Copy link
Member

@leofeyer leofeyer commented Jun 12, 2024

Implements #7091

@leofeyer leofeyer added this to the 5.4 milestone Jun 12, 2024
@leofeyer leofeyer self-assigned this Jun 12, 2024
@leofeyer leofeyer marked this pull request as draft June 12, 2024 08:10
@leofeyer leofeyer linked an issue Jun 12, 2024 that may be closed by this pull request
@leofeyer leofeyer force-pushed the feature/lucide-icons branch 4 times, most recently from 9f4ff04 to ff41bf3 Compare June 12, 2024 11:52
@leofeyer leofeyer marked this pull request as ready for review June 13, 2024 12:32
@leofeyer leofeyer requested a review from a team June 13, 2024 12:32
@leofeyer
Copy link
Member Author

I‘m quite happy with the new icons except for the "edit child records" one. Maybe someone has a better idea regarding both the icon and the color.

@heimseiten
Copy link
Contributor

I like the between-horizontal-start in Contao orange for the child icon.
3

@contaoacademy
Copy link

Will the new icons also be integrated into the 5.3 LTS?

@fritzmg
Copy link
Contributor

fritzmg commented Jun 15, 2024

Will the new icons also be integrated into the 5.3 LTS?

No, it's a new feature.

@aschempp
Copy link
Member

I like it! Ideas/findings

  • try to use the copy-plus icon for copying records
  • try layout-list for child elements
  • try component or package icon for theme modules
  • try save icon for theme export
  • try import icon for theme import
  • try git-compare-arrows for comparing templates as well as comparing versions
  • try clipboard-paste where arrow is not only inside the pasteboard (and adjusted version for paste after)
  • the default tree node icon (e.g. in favorites list or template file) does not look updated
  • the disabled delete icon looks incorrect (see screenshot)
    Bildschirmfoto 2024-06-15 um 20 54 31
  • are template folder intentionally marked with a lock icon?
  • the icon for internal and external redirect look the same? Maybe use something with external-link?

@leofeyer
Copy link
Member Author

leofeyer commented Jun 17, 2024

  • try to use the copy-plus icon for copying records

I‘d rather keep the current icon, because copy-plus is harder to distinguish from "copy with children":


  • try component or package icon for theme modules

The component icon doesn‘t look good next to the layout icon. And I don‘t like the 3D effect of the package icon, because it does not match any other icon. Solved in 840af49.


  • try save icon for theme export
  • try import icon for theme import

Like this?


  • the default tree node icon (e.g. in favorites list or template file) does not look updated

I cannot reproduce. Can you make a screenshot?

  • the disabled delete icon looks incorrect (see screenshot)

I cannot reproduce this, either. Maybe a browser cache problem?


  • are template folder intentionally marked with a lock icon?

They have always been:


  • the icon for internal and external redirect look the same? Maybe use something with external-link?

This is intentional. You only need to know that it is a redirect page; whether the redirect is internal or external is a detail that does not need to be reflected in the status icon.

@fritzmg
Copy link
Contributor

fritzmg commented Jun 17, 2024

This is intentional. You only need to know that it is a redirect page; whether the redirect is internal or external is a detail that does not need to be reflected in the status icon.

I have to disagree here, this is a rather important distinction.

@leofeyer
Copy link
Member Author

No, it is not. We even discussed merging the two page types because it really makes no sense to have both.

@aschempp
Copy link
Member

I like the import export much better this way.

Also noticed this when trying to move a root page, can you reproduce that (the root paste icon is left-aligned)?
Bildschirmfoto 2024-06-17 um 14 58 00

@aschempp
Copy link
Member

Also noticed the plus icon looks really big everywhere, I think it should be similar in size to the eye. That would also look better in the global operation for adding a record (next to the back icon).

Bildschirmfoto 2024-06-17 um 15 16 11

@aschempp
Copy link
Member

  • the default tree node icon (e.g. in favorites list or template file) does not look updated

I cannot reproduce. Can you make a screenshot?

Here's what templates look like for me

Bildschirmfoto 2024-06-17 um 15 40 43

@fritzmg
Copy link
Contributor

fritzmg commented Jun 17, 2024

No, it is not. We even discussed merging the two page types because it really makes no sense to have both.

In the absence of a folder page you typically use the internal redirect redirect page when creating a folder structure in your site structure, for menu items that are only there to contain other menu icons (and show them on hover for the desktop for example). The internal redirect also redirects you automatically to its first child, if no redirect page is selected.

In the external redirect page you have to define any arbitrary URL. It does not have any special logic. Using it as a container as described above makes no sense, as the target URL is mandatory.

Merging the two also does not make any sense to me, at least currently. As long as we have both it also important to be able to visually distinguish between the two. (i.e. having links to external resources, like links to social pages etc., should be clearly distinguishable from the other pages).

@fritzmg
Copy link
Contributor

fritzmg commented Jun 17, 2024

Like this?

Shouldn't it be the other way around? I find it confusing that an arrow pointing away from something actually imports something, and that an arrow pointing towards something exports something.

@asaage
Copy link

asaage commented Jun 17, 2024

Shouldn't it be the other way around? I find it confusing that an arrow pointing away from something actually imports something, and that an arrow pointing towards something exports something.

unless that "something" resembles a local disk 🙈

@asaage
Copy link

asaage commented Jun 17, 2024

The internal redirect also redirects you automatically to its first child, if no redirect page is selected.

essential feature - don't remove that please!

@leofeyer
Copy link
Member Author

I find it confusing that an arrow pointing away from something actually imports something, and that an arrow pointing towards something exports something.

That is because you upload the file to be imported and you download the file to be exported.

@leofeyer
Copy link
Member Author

The internal redirect also redirects you automatically to its first child, if no redirect page is selected.

essential feature - don't remove that please!

There are no plans to remove any functionality! A unified redirect page type could also provide the "redirect to first child" feature.

@leofeyer
Copy link
Member Author

Also noticed this when trying to move a root page, can you reproduce that (the root paste icon is left-aligned)?

Fixed in 408a154.

Also noticed the plus icon looks really big everywhere, I think it should be similar in size to the eye.

It is. You have to compare against its disabled state:

@aschempp
Copy link
Member

I would prefer something like file-spreadsheet for the CSV import icon in table wizard.

Bildschirmfoto 2024-06-19 um 10 49 38

@leofeyer
Copy link
Member Author

I would prefer something like file-spreadsheet for the CSV import icon in table wizard.

For consistency, we should use the same icon as for the other import actions. Fixed in 9254063.

@leofeyer
Copy link
Member Author

Shouldn't it be the other way around? I find it confusing that an arrow pointing away from something actually imports something, and that an arrow pointing towards something exports something.

Changed in b43708b.

@leofeyer
Copy link
Member Author

All suggestions have been addressed. I will now merge the PR so I can start working on the follow-up PRs. If you find any other problem, please create a new issue on GitHub.

@leofeyer leofeyer merged commit aa54c3d into contao:5.x Jun 19, 2024
18 checks passed
@leofeyer leofeyer deleted the feature/lucide-icons branch June 19, 2024 14:23
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.

Use Lucide icons in the back end
6 participants