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(NoteToSelf): add tasks counter #13122

Merged
merged 1 commit into from
Aug 28, 2024
Merged

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Aug 23, 2024

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

Nothing done Partially completed 🏡 Completed
image image image

🏁 Checklist

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Otherwise - looks neat!

src/components/TopBar/TasksCounter.vue Outdated Show resolved Hide resolved
if (!this.$refs.scroller) {
return
}
const tasksDoneCount = this.$refs.scroller.querySelectorAll('.checkbox-content__icon--checked')?.length
Copy link
Contributor

Choose a reason for hiding this comment

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

Another way could be to check message store for messages (- [ ] and - [x] ?

In case there would be an opportunity to render not all messages (virtual scroller)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Performance-wise, it is more expensive as you will loop through messages.
Accuracy-wise, not all - [ ] are considered a checklist, maybe we need to do the same check when clicking on a list item to change the state.

src/components/TopBar/TasksCounter.vue Outdated Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the fix/13034/tasks-counter branch 2 times, most recently from 6c37e2d to 57b5519 Compare August 28, 2024 08:49
@DorraJaouad DorraJaouad force-pushed the fix/13034/tasks-counter branch from 57b5519 to aba6325 Compare August 28, 2024 08:50
@DorraJaouad DorraJaouad merged commit 5690a13 into main Aug 28, 2024
46 checks passed
@DorraJaouad DorraJaouad deleted the fix/13034/tasks-counter branch August 28, 2024 09:33
@Antreesy
Copy link
Contributor

@nickvergessen shall we backport this now, or respecting the string freeze - only after the 20.0 release?

@nickvergessen
Copy link
Member

Drawn apart. I'd say lets do it, but the translation string looks "wrong" anyway

@DorraJaouad
Copy link
Contributor Author

/backport to stable30

Comment on lines 306 to 309
showUpcomingEvent() {
return this.nextEvent && !this.isInCall && !this.isSidebar && !this.isMobile
&& this.conversation.type === CONVERSATION.TYPE.NOTE_TO_SELF
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to be unrelated and breaking upcoming events @DorraJaouad

Copy link
Contributor

@Antreesy Antreesy Sep 6, 2024

Choose a reason for hiding this comment

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

Or supposed to be !==

Copy link
Contributor

Choose a reason for hiding this comment

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

Or supposed to be !==

Still not related to task counting though 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it should be !== indeed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or supposed to be !==

Still not related to task counting though 👀

Also true, but as I was adding the counter and touching that space in TopBar, I thought it could be slipped in this PR.

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

Successfully merging this pull request may close these issues.

Add a counter for checkboxes with a "To Do" title in "Note to self" chat
4 participants