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

fix(NcBreadcrumb): cursor and native title and inline actions rendering text #3927

Merged
merged 5 commits into from
Apr 4, 2023

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Mar 25, 2023

Capture d’écran_2023-03-25_12-54-49

When navigating, clicking the last (current) directory should reload.
You can watch the click using click.native

<NcBreadcrumb v-for="(section, index) in sections"
	:key="section.dir"
	:aria-label="ariaLabel(section)"
	:native-title="ariaLabel(section)"
	v-bind="section"
	@click.native="onClick(section.to)">
	<template v-if="index === 0" #icon>
		<Home :size="20" />
	</template>
</NcBreadcrumb>

@JuliaKirschenheuter, I asked you for review since this touches part of the title change you made on the NcActionLink component

@skjnldsv skjnldsv added bug Something isn't working 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Mar 25, 2023
@skjnldsv skjnldsv requested review from raimund-schluessler and a team March 25, 2023 11:57
@skjnldsv skjnldsv self-assigned this Mar 25, 2023
@skjnldsv skjnldsv requested review from artonge and Pytal and removed request for a team March 25, 2023 11:57
@raimund-schluessler

This comment was marked as resolved.

@raimund-schluessler
Copy link
Contributor

Maybe we should go the same way as in #3617 and add a name prop as replacement for the title prop instead (and make title behave as the native title).

@skjnldsv

This comment was marked as resolved.

@raimund-schluessler

This comment was marked as resolved.

@skjnldsv skjnldsv force-pushed the feat/bread branch 2 times, most recently from df187c5 to 75e155d Compare March 25, 2023 17:27
@skjnldsv
Copy link
Contributor Author

Done @raimund-schluessler. Given the Breadcrumbs also use the NcActions, I had to fix the NcActionLink title usage and provided this to all the other NcActions too 🚀

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv

This comment was marked as resolved.

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 25, 2023
@raimund-schluessler
Copy link
Contributor

Done @raimund-schluessler. Given the Breadcrumbs also use the NcActions, I had to fix the NcActionLink title usage and provided this to all the other NcActions too rocket

Well, that escalated quickly 🙈

Hum, because title is too much bound to the NcAction*, I don't think I can achieve that without being a breaking change. Because it means if I fallback the name from title, then the title will always be defined and the actions will always be seen like this image

I pushed a commit that I think fixes the issue:
grafik

Please have a look and feel free to discard my changes, in case you think they don't work.

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
…ext rendering

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv changed the title fix(NcBreadcrumb): cursor and native title fix(NcBreadcrumb): cursor and native title and inline actions rendering text Mar 28, 2023
@skjnldsv

This comment was marked as resolved.

@raimund-schluessler

This comment was marked as resolved.

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler

This comment was marked as resolved.

@skjnldsv

This comment was marked as resolved.

Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@skjnldsv skjnldsv disabled auto-merge April 4, 2023 13:08
@skjnldsv skjnldsv merged commit bc83996 into master Apr 4, 2023
@skjnldsv skjnldsv deleted the feat/bread branch April 4, 2023 13:08
@skjnldsv skjnldsv mentioned this pull request Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants