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

Migrate composer action menu icons to material design icons + migrate to Vue buttons #6871

Merged
merged 1 commit into from
Jul 27, 2022

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Jul 13, 2022

@GretaD GretaD changed the title Migrate composer icons to material design icons Migrate composer action menu icons to material design icons Jul 13, 2022
@GretaD GretaD self-assigned this Jul 13, 2022
@GretaD GretaD requested a review from ChristophWurst July 13, 2022 11:31
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Why did you rename button to Button? the nc/vue button component is imported as VButton

Are the button changes even necessary for the icon replacement?

@GretaD
Copy link
Contributor Author

GretaD commented Jul 13, 2022

Why did you rename button to Button? the nc/vue button component is imported as VButton

Are the button changes even necessary for the icon replacement?

no there are not necessary on this pr because < button > doesnt have an icon. When it has an icon then it needs < Button >. I did it here because ive done it with other PRs

@ChristophWurst ChristophWurst changed the title Migrate composer action menu icons to material design icons Migrate composer action menu icons to material design icons + migrate to Vue buttons Jul 13, 2022
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 🐘

@GretaD GretaD force-pushed the migrate/composer-icons branch from 69f3463 to 91d9303 Compare July 14, 2022 14:27
@miaulalala miaulalala merged commit b37e4d0 into main Jul 27, 2022
@miaulalala miaulalala deleted the migrate/composer-icons branch July 27, 2022 12:38
@raimund-schluessler
Copy link
Member

There is quite a bit of chaos with the buttons now, you have button, Button and VButton, whereas they all should be VButton (or how you want to import them) regardless of whether they have icons or not. button will only render to a standard HTML component and not profit from the accessibility features of the vue component.

button (standard HTML button with an icon even):

<button
:title="t('mail','Toggle recipients list mode')"
:class="{'active':!autoLimit}"
@click.prevent="toggleViewMode">
<UnfoldMoreHorizontal v-if="autoLimit" :size="24" />
<UnfoldLessHorizontal v-else :size="24" />
</button>

Button (probably also standard HTML with an icon):

<Button :disabled="!canSend"
class="button primary send-button"
type="submit"
@click="onSend">
<Send
:title="submitButtonTitle"
:size="20" />
{{ submitButtonTitle }}
</Button>

Button without icon:

<Button @click="state = STATES.EDITING">
{{ t('mail', 'Go back') }}
</Button>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants