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.

Sign up for GitHub

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

Jump to bottom

Fix actions reactivity #585

Merged
merged 1 commit into from
Sep 12, 2019
Merged

Fix actions reactivity #585

merged 1 commit into from
Sep 12, 2019

Conversation

skjnldsv
Copy link
Contributor

@skjnldsv skjnldsv commented Sep 10, 2019 •
edited
Loading

Reactivities fixes:

  1. When switching from 1 to 2+ items shown, the component was not updating
  2. When only one action and switching the props, none were updating
  3. Fixed aria-label
  4. Cleaned up a bit

I now added $children in the data, that way vue inject its watcher and we get a reactive $children

Examples

<Actions>
	<ActionLink :icon="toggle ? 'icon-external' : 'icon-add'"
		title="Link" href="https://nextcloud.com" />
	<ActionLink  v-if="toggle" icon="icon-external" href="https://nextcloud.com" />
</Actions>
<button @click="toggle = !toggle">Toggle</button>
<Actions>
	<ActionLink :icon="toggle ? 'icon-external' : 'icon-add'"
		title="Link" href="https://nextcloud.com" />
</Actions>
<button @click="toggle = !toggle">Toggle</button>

@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Sep 10, 2019
@skjnldsv skjnldsv added this to the 0.13.0 milestone Sep 10, 2019
@skjnldsv skjnldsv self-assigned this Sep 10, 2019
@skjnldsv skjnldsv added feature: actions Related to the actions components high High priority labels Sep 10, 2019
@skjnldsv
Copy link
Contributor Author

/backport to stable0-12

@skjnldsv
Fix actions reactivity
7cc3f40 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
ChristophWurst
ChristophWurst approved these changes Sep 12, 2019
View reviewed changes
Copy link
Contributor

@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.

Code looks good!

@skjnldsv skjnldsv merged commit a5d4292 into master Sep 12, 2019
@skjnldsv skjnldsv deleted the fix/actions/reactivity branch September 12, 2019 10:16
@backportbot-nextcloud
Copy link

The backport to stable0-12 failed. Please do this backport manually.

@skjnldsv skjnldsv mentioned this pull request Sep 12, 2019
Merged
@skjnldsv
Copy link
Contributor Author

Backport in #588

raimund-schluessler
raimund-schluessler reviewed Jul 24, 2022
View reviewed changes
src/components/Actions/Actions.vue
isValidSingleAction() {
return this.actions.length === 1
&& this.firstActionElement !== null
Copy link
Contributor

Choose a reason for hiding this comment

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

this.firstActionElement is always undefined (it's never defined anywhere after this PR). Should be this.firstActionBinding probably. I'll create a PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

PR in #2878

@raimund-schluessler raimund-schluessler mentioned this pull request Jul 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raimund-schluessler raimund-schluessler raimund-schluessler left review comments

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@korelstar korelstar Awaiting requested review from korelstar

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

Assignees

@skjnldsv skjnldsv

Labels
3. to review Waiting for reviews feature: actions Related to the actions components high High priority
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@skjnldsv @ChristophWurst @raimund-schluessler