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(Todoist): Project with sections to get project name #2151

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

Conversation

tomcat0090
Copy link
Contributor

@tomcat0090 tomcat0090 commented Jan 17, 2023

Projects with segments cannot get the project name.

As a side note, if the project name is not registered in Toggl and the segment name matches the project name in Toggl, it will be recognized as the project name.
This commit is for the list only, and the task detail screen will be fixed separately.

Please remember the Contributing Guidelines ❤️

🌟 What does this PR do?

Resolves some of the problems caused by the addition of sections to Todoist.

For the toggle buttons in the following views, the process of obtaining the project name has been corrected.

  • Today
  • Upcoming
  • Filters & Labels

This will resolve the following Issue.

Issue with applying projects for tasks within a project segment in Todoist #2141
Projects not recognized in Today/Upcoming view if task is part of a section #1930

🐛 Recommendations for testing

All changes should be tested across Chrome and Firefox. -> Done

📝 Links to relevant issues or information

Fixes #2141 #1930

Projects with segments cannot get the project name.
As a side note, if the project name is not registered in Toggl and the segment name matches the project name in Toggl, it will be recognized as the project name.
This commit is for the list only, and the task detail screen will be fixed separately.
return str;
}

function isExist(str, arr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
function isExist(str, arr) {
function contains(str, arr) {

Better naming. and should swap arguments too: contains(arr, str)

or use native Array.includes:

@glensc
Copy link
Contributor

glensc commented Jan 19, 2023

add to PR title the name of what integration you are fixing

@tomcat0090 tomcat0090 changed the title Fix failure to get project name with segments Fix project with sections to get project name Jan 20, 2023
@tomcat0090
Copy link
Contributor Author

Thanks for the review @glensc.

The title has been changed to correct the areas that were pointed out.
This pull request solves the problem of incorrectly retrieving the project name depending on the section.
It also includes the ability to determine that the section name in Todoist is the project name in Toggl. (Of course, the Todoist project name takes precedence.)
Please review again.
If there is any problem, please point it out again.

@glensc
Copy link
Contributor

glensc commented Jan 20, 2023

@tomcat0090 what i had in mind that you include integration name in title, this project (toggl/track-extension) includes many integrations not just integration to "todoist"

@tomcat0090 tomcat0090 changed the title Fix project with sections to get project name Fix project with sections to get project name in Todoist Jan 20, 2023
@tomcat0090
Copy link
Contributor Author

@glensc
I see!
fix it.

@tomcat0090 tomcat0090 changed the title Fix project with sections to get project name in Todoist fix(Todoist): Project with sections to get project name Jan 27, 2023
@tomcat0090
Copy link
Contributor Author

Are there other problems?
@glensc

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

Successfully merging this pull request may close these issues.

Issue with applying projects for tasks within a project segment in Todoist
2 participants