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.
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
[docs] Improve documentation of the system #23294
[docs] Improve documentation of the system #23294
Changes from 10 commits
eb5426b
4cc8ceb
4ce7687
e99de3b
cae9761
54d8a55
b8fb9dd
9b56867
e5978a4
d99cf00
c94ca93
79a7d8b
e07060a
acbc866
e2652f9
a1bafa7
f8e965d
aa081b8
b30e24b
14a908c
c029718
682a5a8
cb867a9
52720d5
dbd5026
35fe80e
c6a8d46
7fc6a78
d909c63
a19edd6
e28a683
87c118e
b0fa09c
e3808bb
01bc241
780d5fd
cc5f8c7
6a9108d
9d5bd14
2854661
8b7ea10
a34ae16
8c5823a
d50a6af
28eb893
b198753
6a8b944
64b51cc
47af380
18752f5
541d0b7
af31b6c
a9ca97f
dd7b9fa
5796085
34c4309
304116c
faee8de
71575b8
584b289
602d1d7
153a18d
6d66c80
16a1c95
357f73e
b824976
6a4630b
3101abd
9187099
90ba086
0b0670c
40fe4a7
cffb670
725c552
c0f2269
5514a6b
5e7fa1a
873490d
fb42502
cb35c1e
0f9a565
6d1c063
05b40fd
efab4f0
762b4cc
5924891
2ad356c
6007e3b
da64fea
5aa6192
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I would start with mentioning
@material-ui/core
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of this:
Later when we support this on all primitives, we can add that here too. Or at least, we can export the
styleFunctionSx
and add something like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'l wait for it to be updated before correcting this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't decided on adding
sx
to every component nor have we released it. We shouldn't document something before.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Slider is meant to be a projection of how all the other components will be architectured. Slider now supports the sx prop and was released. So I think that in theory, this decision and release event has already happened. I would argue that the decision started 2 years ago with
https://medium.com/material-ui/introducing-material-ui-design-system-93e921beb8df
"look into" as how can we make it fast enough. Then about a year ago with #15561.
Having a look at all the issues that where closed linking to this issue (#15561) is interesting to gauge the pain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is not how an alpha works. Its goal is to give us freedom to add and remove things without having to release a major version every time. If you're now flipped to "everything released in alpha is stable" then you should've told us that before we started working on v5. That's a pretty fundamental change to the alpha process especially if it involves decisions we haven't discussed.
As ususal: something existing is in no way shape or form under any circumstance and argument for its existence. Slider with sx is not a good API just because it shipped with it. The original PR still doesn't include a real world use case and just some toying around.
A decision isn't a process. Could you link me the conclusion of the discussion with having widgets with system props that just affect the root container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, the objective of pre-releases is to allow a short feedback loop. It allows us to validate ideas directly with developers. On this one, we are waiting to get insight from developers after having released it. It seems that the trend of improvement is toward supporting
sx
everywhere: #23220. The value of the prop increases the more it can be used in a systematic manner.I don't understand the concern with the
sx
prop, the value proposition started to be discussed in #22819, then #23053, e.g. #23053 (comment).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's JavaScript. JavaScript is only good if it solves a problem. On the flipside: I do not understand how you don't see a problem with adding features after maintaining open-source for 3+ years.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, it makes sense, so I would summarize the concern into: is this going in the right direction, and is the opportunity cost right? From my perspective, the answer is yes to both of these questions. I would personally use it over what we have now in future projects. It's really meant to be Bootstrap's utility equivalent (but covering more use cases).
I guess we can keep it in the alpha state longer to make sure we don't regret it?