DataViews: centralize the view definition and rename list to table
#56693
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oandregal
requested review from
ntsekouras,
youknowriad,
jameskoster and
jorgefilipecosta
oandregal
added
[Type] Experimental
oandregal
commented
Nov 30, 2023
| export const LAYOUT_GRID = 'grid'; | ||
| export const LAYOUT_SIDE_BY_SIDE = 'side-by-side'; | ||
|
|
||
| export const VIEW_LAYOUTS = [ |
There was a problem hiding this comment.
This exercise has shown that there's quite a bit of expectations defined for a view, already. At some point in the following weeks, we could explore having official APIs for some things that were copied around and are now constants.
For example:
getIconForViewType( )viewTypeSupports( 'preview' )- etc.
|
Size Change: -137 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Nov 30, 2023
There was a problem hiding this comment.
I like this PR 👍 Good start. We definitely need things like getLayout( layout ). I saw the icon used in the sidebar and it could use such API.
I also think we should start thinking about moving everything to a "dataviews" package. It could contain a store with layout registry for instance...
|
If 'side-by-side' is to become 'list', then renaming the current 'list' to 'table' seems reasonable. If that's the case we can restore the |
Thanks for the confirmation! I'll do that then. |
oandregal
force-pushed
the
update/view-layouts-reorganize
branch
from
a8c9b36
to
0d2d010
Compare
|
ok, this is ready then:
Gravacao.do.ecra.2023-12-01.as.09.44.57.movOne thing I haven't done in this PR is renaming the view components. I'll do that separately so it's easier to review. |
|
follow-up renaming components #56709 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #55083
What?
This PR:
listlayout totable& updates its icon toblockTableside-by-sidelayout tolistGravacao.do.ecra.2023-12-01.as.09.44.57.mov
Why?
The centralization helps to visualize the emerging shape of a view object, as well as helping to maintain consistency (see for example how icons drifted due to lacking a single place to update them all: #56602).
Renaming was part of the tracking issue.
How?
Centralizes the view definition in
constants.js.Testing Instructions
Follow-ups