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

Enhancements for ember 4 #31

Open
wants to merge 56 commits into
base: master
Choose a base branch
from
Open

Conversation

madnificent
Copy link
Member

The Ember framework has evolved substantially since we first constructed the ember-data-table. When this component was envisioned, Data Down Actions Up (DDAU) was not a real thing yet and variables were passed with two-way binding. We were discovering something like named blocks would be handy but we did not have solutions.

We now have tracked properties, DDAU, named blocks and Ember is moving towards being more explicit about integrations.

It has been clear for a while that the ember-data-table could be upgraded to cope with this new reality. We have also noticed that we'd prefer to expose more basic building blocks so alternative strategies can be implemented (like cards). This PR works towards upgrading the ember-data-table to modern Ember with named blocks. It also gets rid of the mixins that are currently in place. This has been a fairly mechanical effort up to this point and improvements can be made. Some things we see:

  1. the code likely has bugs due to mechanical replacement
  2. naming of the components can be cleaned up (the named block and the component name are not shared)
  3. some libraries can be stripped, as suggested in Ember v4 support #30
  4. arguments (and their names) provided through the tree can likely be improved
  5. for docs, the RawDataTable component could serve as an example to make data-table flavours use the same API (but upgrades will be manual)

A PoC bootstrap ember-data-table has been built based on the included raw-data-table.hbs file. It would be wise to have a few more of these cases so we can see if other arguments would need to be added.

This conversion cannot be checked in its functionality, as we must
convert the underlying components too in order to check the current
setup.
The NumberPagination component now also consumes content and sends
updates.
This is the path that allows us to render a sortable header.  Yet
nothing else is currently being rendered.
The content can now be rendered in an Ember 4.x application.
The base building blocks the search used are quite neat.  We've
continued to use those and were able to simplify a tiny bit due to new
abstractions.  This component can likely benefit from some shuffling of
the code but that would require a second reading.
Filter was not passed correctly.
These had some leftover todos attached to them.  Not needed.
The test cases may help us find a way out of this, but this feels like
it's a sensible step to at least get the branch into a possibly sensible
state.
This provides with a few options, as summarized in the docstrings.
This component is the first to receive support for named blocks.  This
allows a user to override only part of the component.

The pattern that arrizes in the component is feasible to stare through
and so we hope this may be a construction that we could iterate on for
the other components too.
This should allow users to use the menu once again.  You don't need to
extend the full component for using this anymore :)
The idea is that we allow various slots to be used to override parts of
the data table, allowing other parts to stay put.  This structured could
possibly be used as a guideline to styled versions of the
ember-data-table.
The big template would make replacing parts easier from a consumer's
point of view.  It does mean we have a single very large template with
all visible options embedded in it.  Question would then be if this is
easier to maintain and manage than the alternative with many components.
This approach should make the template a tad easier to understand as we
have removed more information-passing logic into components that don't
render output themselves.
These basics should help people get started with the components.  The
route and controller could receive more love to allow better combination
with other libraries.  Not sure how much that happens at this point in
time.
Whilst implementing an alternative view, some thing options turned out
to be missing.  Adding those in.
These are space-separated lists you may supply to indicate for which
columns the :data-header and :data-cell should be rendered.  When
specified it will only be rendered for those cells.
This DSL should make it obvious how to add common views to be linked at
the end of the view.  It is possible to override the statement by using
an actions block.
Renaming the files ensures they don't currently run.  We may want to
alter these tests so they're picked up again later.
The number pagination has a concept of the server pagination on the
outside and human pagination on the inside.  Some features could be
added, but this is an easier to understand workflow for the features
that exist now.
@nvdk
Copy link
Member

nvdk commented Feb 16, 2022

Could this be released as a beta so it's easier to experiment with in our apps?

@madnificent
Copy link
Member Author

Yes, no, maybe, sure, not yet but maybe (*)? I think it makes most sense to try converting a few things with everything linked so we can update the renderless components at the same time. First tests show there's some streamlining of variables we should execute (these used to be internal state but are now much more a public API), and we expect some bugs to be in here.

I'm good with setting up a build for this if you'd want to gamble and run it somewhere, but I'm even more game for hacking on your use-case and seeing if it fits and where the friction is. I'm fairly certain the API is not stable as it stands, though the concept would stay the same and can easily be evaluated. If the concept stands, most changes will be name changes.

This is an open response, any thoughts? Go for a build anyways? Go yolo and base ourselves on a git tag?

@nvdk
Copy link
Member

nvdk commented Apr 27, 2022

Ah I think I misunderstood how mature this was, perhaps a release is a bit early in that case.

FYI: From experience ember addons often have issues when installed from a git tag because npm tends to run the install scripts a bit different when doing that (I believe the prepare or prepublish step isn't ran or something like that, I'm sure there are good reasons for that but 🤯 ). That may or may not be an issue with this addon.

The components hbs files mostly follow a logical name but it can still
be a bit confusing.  We're adding the breadcrumbs now so we can find our
way back more easily.
Cleaner wording, replaced semi-automatically.

Co-author: @erikap
More obvious naming.

Co-author: @erikap
A warning is shown on missing update functions.  This will not error
before they're used but it's a bit more helpful.

Co-author: @erikap
This only appears in the RawDataTable component at this point.  Best to
update it as it will likely become the preferred starting point.
These properties have been replaced with the fields object.

Co-author: @erikap
erikap and others added 18 commits July 19, 2024 11:56
Uses the human element towards the implementor which hopefully makes
things a bit easier to implement.

Co-author: @erikap
The readme would need to be completely revamped for the new data table.
This should go a long way towards that.
This documents the various parts which can be overridden.  It's probably
the easiest to check the specific design implementation but this might
help determine the intent.
These were within a component and they're not allowed there.
This is a wrapper object containing:
- size
- page
- filter
- sort
- isLoading
- updatePage
- updatePageSize
- updateFilter
- updateSort

It is an opaque object from the user's point of view.  All current
arguments take precedence making it easy to override.  The default case
becomes shorter which is great to show what's special about the specific
Data Table at hand and thus lowers mental overhead for the default case.
Makes it easier for users to fallback to the default behaviour when
implementing custom sort parameters for an attribute.
The addon imports the JSONAPI serializer from ember-data
@erikap erikap force-pushed the feature/enhance-for-ember-4 branch from 1842fa6 to c56e562 Compare October 16, 2024 12:56
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.

3 participants