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

feat(VAlert): better aligment with prepend icon #20700

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

J-Sek
Copy link
Contributor

@J-Sek J-Sek commented Nov 14, 2024

Description

resolves #20636

Markup:

Playground
<template>
  <v-app>
    <v-container :max-width="650" class="space-y-6">
      <div class="d-flex ga-6">
        <v-switch hide-details color="primary" v-model="dense" label="compact" />
        <v-switch hide-details color="primary" v-model="prominent" label="prominent" />
        <v-switch hide-details color="primary" v-model="closable" label="closable" />
      </div>

      <v-alert v-bind='alertConfigWithTitle' />
      <v-alert v-bind='alertConfigWithTitleAndText' />
      <v-alert v-bind='alertConfigWithShortText' />
      <v-alert v-bind='alertConfigWithLongText' />

      <v-number-input hide-details class="mt-6" density="compact" variant="outlined" v-model="iconSize" label="icon size" max-width="200" suffix="px" />

      <v-alert v-bind='alertConfigWithTitle' :icon-size="iconSize" />
      <v-alert v-bind='alertConfigWithTitleAndText' :icon-size="iconSize" />
      <v-alert v-bind='alertConfigWithShortText' :icon-size="iconSize" />
      <v-alert v-bind='alertConfigWithLongText' :icon-size="iconSize" />

    </v-container>
  </v-app>
</template>

<script setup>
  import { computed, ref } from 'vue'
  const dense = ref(false)
  const closable = ref(false)
  const prominent = ref(false)
  const iconSize = ref(24)

  const alertConfig = computed(() => ({
    icon: 'mdi-adjust',
    density: dense.value ? 'compact' : undefined,
    closable: closable.value,
    prominent: prominent.value
  }))
  const alertConfigWithTitle = computed(() => ({
    ...alertConfig.value,
    title: 'Past date not allowed',
  }))
  const alertConfigWithTitleAndText = computed(() => ({
    ...alertConfigWithTitle.value,
    text: 'You requested date that is in the past. Please adjust your selection.',
  }))
  const alertConfigWithShortText = computed(() => ({
    ...alertConfig.value,
    text: 'You requested date that is in the past. Please adjust your selection.',
  }))
  const alertConfigWithLongText = computed(() => ({
    ...alertConfig.value,
    text: 'You requested date that is in the past. Please adjust your selection. More text more text more text.',
  }))
</script>

<style>
  .space-y-6 > * + * {
    margin-top: 24px
  }
  /* outline to help us see alignment */
  .v-alert .v-alert__prepend {
    position: relative;
  }
  .v-alert .v-alert__prepend:after {
    pointer-events: none;
    position: absolute;
    display: block;
    content: '';
    inset: 0;
    width: 999px;
    border: 1px #0008 dashed;
  }
</style>

@J-Sek J-Sek self-assigned this Nov 14, 2024
@J-Sek J-Sek added T: enhancement Functionality that enhances existing features C: VAlert labels Nov 16, 2024
Comment on lines +63 to +66
iconSize: {
type: Number,
default: null,
},
Copy link
Member

Choose a reason for hiding this comment

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

If we're going to add a new prop like this, we would need to do a few things:

  • Switch target from master to dev
  • Discuss where else this could be used if anywhere

Copy link
Contributor Author

@J-Sek J-Sek Jan 23, 2025

Choose a reason for hiding this comment

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

Other places usually respond to either straight or nested defaults { ... VIcon: { size: 20 } } or Sass variables.

VAlert is overriding it because of prominent flag. I wish it would do either zoom: 2 or transform: scale(2) in CSS instead. I am also not sure why it was bumped from 24 to 28 between v2 and v3.

There is only one more similar case I see: VSwitch #thumb slot. But it is a very niche use case.

@J-Sek J-Sek changed the base branch from master to dev January 23, 2025 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VAlert T: enhancement Functionality that enhances existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Better support for custom icon size in VAlert
2 participants