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

Fix tabular headers #17799

Merged
merged 16 commits into from
Apr 11, 2023
Merged

Fix tabular headers #17799

merged 16 commits into from
Apr 11, 2023

Conversation

Nitwel
Copy link
Member

@Nitwel Nitwel commented Mar 15, 2023

Description

This PR reworks the way the tabular layout works with aliased requests, as the old way was really hacky by trying to merge aliases depending on their originally name. The implementation now only ever passes the the data a certain column had requested back to it so there is no collision in the data each display requires.

And the other thing that got improved is that now only nested fields of translations are rendered in a special way.

Fixes #17716
Fixes #17506
Fixes #17670
Fixes ENG-690

@Nitwel Nitwel requested a review from a team March 15, 2023 08:56
@Nitwel Nitwel self-assigned this Mar 15, 2023
@Nitwel Nitwel requested review from paescuj, licitdev and azrikahar and removed request for a team March 15, 2023 08:56
@paescuj

This comment was marked as resolved.

@Nitwel Nitwel marked this pull request as ready for review March 16, 2023 15:11
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

Only a small comment, otherwise looks great 😃👍

app/src/layouts/tabular/tabular.vue Outdated Show resolved Hide resolved
app/src/layouts/tabular/tabular.vue Outdated Show resolved Hide resolved
Copy link
Member

@paescuj paescuj left a comment

Choose a reason for hiding this comment

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

Small addition:

The value passed to render-display (computed in getDisplayValue) is now an array in case of relational fields. While this doesn't seem to be an issue, the following warning is shown in the console:

[Vue warn]: Invalid prop: type check failed for prop "value". Expected String | Number, got Array  
  at <FormattedValue interface="input" interface-options=null value= 
Array [ "test" ]
  ... > 
  at <VErrorBoundary key=2 name="display-formatted-value" > 
  at <RenderDisplay value= 
Array [ "test" ]
 display="formatted-value" options=null  ... > 
  at <TableRow headers= 
Array(3) [ {…}, {…}, {…} ]
 item= 
Object { category: "category1", id: 1, 1be40: (1) […], 5622e2d6: (1) […] }
 show-select="multiple"  ... > 
  at <Draggable key=3 modelValue= 
Array [ {…} ]
 onUpdate:modelValue=fn  ... > 
  at <VTable key=0 ref="table" modelValue= 
Array []
  ... > 
  at <Tabular key=1 class="layout" tableHeaders= 
Array(3) [ {…}, {…}, {…} ]
  ... > 
  at <PrivateView key=1 title="Parent" small-header=undefined > 
  at <AsyncComponentWrapper key=1 title="Parent" small-header=undefined > 
  at <TabularWrapper ref="layoutRef" selection= 
Array []
 onUpdate:selection=fn  ... > 
  at <ContentCollection collection="parent" bookmark=null archive=null > 
  at <CollectionOrItem collection="parent" bookmark=undefined archive=null  ... > 
  at <RouterView> 
  at <RouterPassthrough onVnodeUnmounted=fn<onVnodeUnmounted> ref=Ref< 
#text ""
 > > 
  at <RouterView> 
  at <RouterPassthrough onVnodeUnmounted=fn<onVnodeUnmounted> ref=Ref< 
#text ""
 > > 
  at <RouterView key=1 > 
  at <App> chunk-2GTISQ3F.js:1641:13

So we can probably just extend


by Array, correct?

Nitwel and others added 3 commits March 27, 2023 17:37
Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
@Nitwel
Copy link
Member Author

Nitwel commented Mar 27, 2023

Added Array to the formatted-value display. 👍

@rijkvanzanten rijkvanzanten merged commit ea35a1a into main Apr 11, 2023
@rijkvanzanten rijkvanzanten deleted the fix-17716 branch April 11, 2023 14:22
@rijkvanzanten rijkvanzanten added this to the Next Release milestone Apr 13, 2023
Giupest added a commit to apuliasoft/directus that referenced this pull request Apr 14, 2023
@hanneskuettner
Copy link
Member

This PR caused the broken calendar layout display template #18206 and will also break the CSV export as it changed the return type (and function as far as I can tell) of useAliasFields.

In both cases the expected key is aliasField which has been removed.

See

const { aliasFields } = useAliasFields(ref(fields));

and

const { aliasFields } = useAliasFields(ref(fields));

hanneskuettner pushed a commit to hanneskuettner/directus that referenced this pull request Apr 18, 2023
* Rework table headers

* fix tabular headers

* run linter

* fix display with translations

* remove console log

* run linter

* Update app/src/layouts/tabular/tabular.vue

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>

* Update app/src/layouts/tabular/tabular.vue

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>

* add array prop

* turn to dealiased

---------

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>
Co-authored-by: Azri Kahar <42867097+azrikahar@users.noreply.github.com>
@br41nslug br41nslug mentioned this pull request Apr 20, 2023
6 tasks
meditadvisors pushed a commit to ciso360ai/directus-mod that referenced this pull request May 13, 2023
* Rework table headers

* fix tabular headers

* run linter

* fix display with translations

* remove console log

* run linter

* Update app/src/layouts/tabular/tabular.vue

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>

* Update app/src/layouts/tabular/tabular.vue

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>

* add array prop

* turn to dealiased

---------

Co-authored-by: Pascal Jufer <pascal-jufer@bluewin.ch>
Co-authored-by: Rijk van Zanten <rijkvanzanten@me.com>
Co-authored-by: Azri Kahar <42867097+azrikahar@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
5 participants