Skip to content

[4.0] Convert jgrid if (icon) to switch (icon)#30482

Closed
N6REJ wants to merge 2 commits intojoomla:4.0-devfrom
N6REJ:jgrid-to-switch
Closed

[4.0] Convert jgrid if (icon) to switch (icon)#30482
N6REJ wants to merge 2 commits intojoomla:4.0-devfrom
N6REJ:jgrid-to-switch

Conversation

@N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Aug 26, 2020

Pull Request for Issue # .

Summary of Changes

converts multiple if statements into switch statements.

Testing Instructions

inspect administrator/index.php?option=com_content&view=articles
verify icons display properly using fas fa- in class names.
apply pr.
verify icons still use fas fa- in class names and display the same icons.

Actual result BEFORE applying this Pull Request

image

Expected result AFTER applying this Pull Request

image

Documentation Changes Required

none

@N6REJ
Copy link
Contributor Author

N6REJ commented Aug 26, 2020

@hans2103 how's this look?

@N6REJ
Copy link
Contributor Author

N6REJ commented Aug 26, 2020

@infograf768 can you try your 3rd party app that had icon issues before and let me know if it still works?

Copy link
Contributor

@hans2103 hans2103 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@N6REJ Looking good!
Tested the changes successfully

@hans2103
Copy link
Contributor

I have tested this item ✅ successfully on c2057ce


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30482.

@N6REJ N6REJ marked this pull request as ready for review August 26, 2020 06:45
@N6REJ N6REJ changed the title [4.0][DRAFT] convert jgrid if (icon) to switch (icon) [4.0] Convert jgrid if (icon) to switch (icon) Aug 26, 2020
@sandewt
Copy link
Contributor

sandewt commented Aug 26, 2020

I have tested this item ✅ successfully on c2057ce


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30482.

@infograf768
Copy link
Member

@infograf768 can you try your 3rd party app that had icon issues before and let me know if it still works?

Looks like this has no impact on com_localise.

@N6REJ
Copy link
Contributor Author

N6REJ commented Aug 26, 2020

@Quy not at all, we'll be making more pr's as we progress.. @hans2103 will merge my changes and his as we complete the conversions.

@N6REJ
Copy link
Contributor Author

N6REJ commented Aug 31, 2020

closing since we've merged it now #30470

@N6REJ N6REJ closed this Aug 31, 2020
@N6REJ N6REJ deleted the jgrid-to-switch branch August 31, 2020 15: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.

5 participants

Comments