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

Add Caption component #1740

Closed

Conversation

marcoambrosini
Copy link
Contributor

Screenshot from 2021-03-05 09-51-00

Signed-off-by: Marco Ambrosini marcoambrosini@pm.me

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
@marcoambrosini marcoambrosini added 3. to review Waiting for reviews component Component discussion and/or suggestion labels Mar 5, 2021
@marcoambrosini marcoambrosini self-assigned this Mar 5, 2021
-->

<template>
<div role="heading" class="app-navigation-caption">
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused by this. We already have a Caption component for the navigation.
Is this component navigation only? It's also missing documentation.

I think the name should be more straightforward, and maybe the component should be in a dedicated section of the docs? Like 'Text', where we could also put the highlight component as well? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this component navigation only

It's used in both navigation and sidebar in Talk

It's also missing documentation

Oopsss

I think the name should be more straightforward, and maybe the component should be in a dedicated section of the docs? Like 'Text', where we could also put the highlight component as well? thinking

Not sure I get this, quick call?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I get this, quick call?

For transparency and posterity it would be cool to have the infos, decisions and so on here with the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ma12-co what was the results of the call ? Can you summarize ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waiting for @jancborchardt
Discussion here #1751

@PVince81
Copy link
Contributor

PVince81 commented Mar 8, 2021

@ma12-co what's the difference with your other similarly titled PR ? #1739

@marcoambrosini
Copy link
Contributor Author

@ma12-co what's the difference with your other similarly titled PR ? #1739

That component is for use within actions only.

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 19, 2021
@marcoambrosini
Copy link
Contributor Author

closing in favor of #1863

@ChristophWurst ChristophWurst deleted the upstream-AppNavigationCaption-component branch April 19, 2021 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress component Component discussion and/or suggestion technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants