Skip to content

Conversation

@ildar-min
Copy link
Collaborator

Type of change

  • Documentation (updates to the documentation, readme or JSDoc annotations)
  • New feature (a non-breaking change that adds functionality)

Description

Implemented ribbon component

@ildar-min ildar-min requested a review from leonid as a code owner December 16, 2024 09:45
* The alert type
*/
type: 'info' | 'success' | 'warning' | 'critical' | 'error'
type: 'info' | 'success' | 'warning' | 'critical' | 'danger' | 'neutral'
Copy link
Member

Choose a reason for hiding this comment

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

You can use export type AcvAlertRibbonVariant = typeof ALERT_RIBBON_VARIANT[keyof typeof ALERT_RIBBON_VARIANT];
Or the same type from Alert component

Copy link
Member

Choose a reason for hiding this comment

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

or rename it to TYPE

* The alert type
*/
type: 'info' | 'success' | 'warning' | 'critical' | 'error'
type: 'info' | 'success' | 'warning' | 'critical' | 'danger' | 'neutral'
Copy link
Member

Choose a reason for hiding this comment

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

or rename it to TYPE

</template>

<style scoped>
<style lang="scss" scoped>
Copy link
Member

Choose a reason for hiding this comment

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

we dont use scss in components styles

@ildar-min ildar-min force-pushed the feat/ribbon branch 5 times, most recently from ca0c61a to c7d6ef8 Compare December 16, 2024 12:15
@ildar-min ildar-min requested a review from leonid December 16, 2024 13:16
* The alert type
*/
type: 'info' | 'success' | 'warning' | 'critical' | 'error'
type: keyof typeof TYPE
Copy link
Member

Choose a reason for hiding this comment

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

i would rather create a separate typescript type to make it available in exported types later

<script setup lang="ts">
import type { AcvRibbonEvents, AcvRibbonLink, AcvRibbonProps, AcvRibbonSlots } from './ribbon.ts';
import Alert from '@/components/alert/alert.vue';
import Button from '@/components/button/button.vue';
Copy link
Member

Choose a reason for hiding this comment

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

Its better to use consistent names in imports ie AcvAlert, AcvButton

@leonid leonid merged commit 6be17b7 into acronis:main Dec 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants