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: add design callouts in all the documentation when incompatibility with ODS #1614
base: main
Are you sure you want to change the base?
docs: add design callouts in all the documentation when incompatibility with ODS #1614
Changes from 54 commits
b8a48d8
fda5e4e
fee6364
867e9a5
aa70535
798f1f3
4a1d317
350c3f9
8b84244
0bfbb44
2112c38
3f3bd2a
33fc463
ab28cf2
885c070
0736e55
dea2716
34a7060
c9426ab
3c5232c
cd0cf7b
fa4be81
2ce1f27
f455984
c55605e
f3706df
13a9d53
6a087c2
8cd8a84
ea144cf
eb3cc22
5e33f1b
eac00f1
5472045
bc01e55
39193ea
de8fc8f
7a4e7e4
74a336e
b819d38
f9b896e
2e9b718
b19eb41
5d3b634
776708e
4480ba8
20e6a3a
512eca4
8c550e9
2443926
d59bb75
e396a16
15c1a80
f8936ce
244f780
7298b38
a72a977
0be5ae2
651d6ed
454457a
83942cc
28b2ce7
fda225f
62c76a4
7b7c035
ed643d8
de08a1c
57faa2d
36716cf
5632d11
4fcd51d
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.
IDK why we would remove this specific sentence here.
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.
Because we don't want users to do so. We want them to use the component the way it is in Boosted.
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.
Feel like the concepts behind will be lost. Same below.
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.
Once again, the problem is not the technical part, but the look of the component used to illustrate this technical part...
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 should mention it or maybe change the examples look but I'm not sure this is possible...
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.
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.
Linked to my previous comment. If this is only a design reason but that the feature can be used it should be clearly explained.
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. From the info I have so far, the problem is the fact that the design should be buttons and not links.
TBH, it would be good to re-clarify the reasons of not using with the design.
Meanwhile, what about:
With more details, it might be useful to add a "refer to our Boosted ..." and/or "refer to the xx guidelines the ODS website"
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 use links on the whole page here even for https://deploy-preview-1614--boosted.netlify.app/docs/5.2/components/navs-tabs/#nested-tabs. Imo, it's not the main issue here since it's a design issue + no need to redirect to tabs light imo
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.
Let me try to clarify:
<a>
that have the look of buttons<button>
. Here, it's not possible to have that. If we want to have components that have the look of buttons, then in the code they should be<button>
html tags.Would this version be more satisfying ?
These link variants, which are just examples illustrating the use of the fill and justify utilities, should not be used because they do not respect the Orange Design System specifications. Indeed, nav tabs and html tags should not look like buttons.
Instead, please consider using our Boosted Tabs light variant. You can also refer to Navigation guidelines on the Orange Design System website.
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 strongly disagree with the fact that
<a>
shouldn't look like buttons. Since many call to action are presented as buttons but are truly links. I think it strongly depends on the context and the associated action. IMO, the actual behavior is fine.Otherwise, many many and many designs will have to change as well.
IMO, if this is accepted by design:
![image](https://user-images.githubusercontent.com/91960143/224987053-d79a981a-2f8e-4fde-89c6-828d041bc379.png)
instead of:
![image](https://user-images.githubusercontent.com/91960143/224988080-e8252ef0-f7d6-40ea-810a-13a2a3b3db9e.png)
We might consider using
.nav-underline
instead of.nav-pill
to present the remaining doc. We should keep a callout only if the design is against a specific feature. Any thoughts @julien-deramond ?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.
Just done it.
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.
same as above (just not to forget)
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.
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.
Resolved ?
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.
Same answer as above:
Here, we have a nav with links that have the look of buttons . Here, it's not possible to have that. If we want to have components that have the look of buttons, then in the code they should be html tags.
Now besides the combination "html tag/expected design", the only look that is ok for such components is the one of the tabs light.
Would this version be more satisfying ?
These link variants, which are just examples illustrating the use of the fill and justify utilities, should not be used because they do not respect the Orange Design System specifications. Indeed, nav tabs and
<a>
html tags should not look like buttons.Instead, please consider using our Boosted Tabs light variant. You can also refer to Navigation guidelines on the Orange Design System website.