search

adopted-ember-addons / ember-light-table

Lightweight, contextual component based table for Ember
http://adopted-ember-addons.github.io/ember-light-table/
MIT License
312 stars 131 forks source link

Dummy app updates #788

Closed maxwondercorn closed 2 years ago

RobbieTheWagner commented 2 years ago

@maxwondercorn is this still WIP?

maxwondercorn commented 2 years ago

Ye still WIP. I've converted the dummy app to native classes and still have a couple issues to resolve.

RobbieTheWagner commented 2 years ago

@maxwondercorn I just merged https://github.com/adopted-ember-addons/ember-light-table/pull/757 which unfortunately caused some conflicts here. Sorry about that!

maxwondercorn commented 2 years ago

@rwwagner90 re: the conflicts above. I've already removed all the dummy app computed's with native getters or @tracked properties.

After this commit is merged, you can review and merge the PR. The @classic decorator can be removed once the addon is converted to glimmer components.

Also, the new ember font-awesome add-on is incompatible with how the icons are used in ELT. I think w3 could remove the font-awesome dependencies and still provide the same functionality. It would be a breaking change and I'll dig into it a bit

maxwondercorn commented 2 years ago

Hopefully the final pass. Rebased and reverted font-awesome changes. I still do not see icons in app but this can be addressed later. All app examples work.

RobbieTheWagner commented 2 years ago

@maxwondercorn I think some of the changes I made somehow made it into this PR. Perhaps a bad rebase or an accidental pull after rebasing before force pushing? Or perhaps an issue from me squashing and merging. Would you mind doing one final rebase and dropping the commits that should not be included in this PR please?

maxwondercorn commented 2 years ago

Will look at it tomorrow. Very well could be a bad rebase. Wouldn’t be the first time.

RobbieTheWagner commented 2 years ago

It does seem like font awesome icons are not showing up anymore, as you said. I don't know what changed since I opened my PR converting them. I will work on using the <FaIcon> component after we get this PR merged.

maxwondercorn commented 2 years ago

Glad I'm not too crazy re: the icons 😄. Yes it was a bad rebase. Looks like I pulled partial changes from other merged PR's. Too much work to fix. I just recreated the changes on the latest master and will close this PR.

I'll have a new PR shortly

maxwondercorn commented 2 years ago

Closing in preference of PR #793