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

refactor: replace className() method with addClassName() #2548

Conversation

ilandikov
Copy link
Collaborator

Description

TaskFieldRenderer has a className() method acting like a data getter (get a component's class). The data is used later to add to an HTMLElement. This PR replaces it with a addClassName() method that add the class to the target HTMLElement.

Previously className() was written in mind with usage in tests as well (As a provider of data). However #2545 removed the relation between the data in TaskFieldRenderer and the data in the tests.

Motivation and Context

  • have testable behaviour instead of data in TaskFieldRendrer
  • hide implementation details of adding classes inside TaskFieldRenderer
  • have uniform behaviour in TaskFieldRenderer: addClassName(), addDataAttribute(), ...

How has this been tested?

New unit tests, breaking the code.

Types of changes

Internal changes:

  • Refactor (prefix: refactor - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
  • Tests (prefix: test - additions and improvements to unit tests and the smoke tests)

Checklist

  • My code follows the code style of this project and passes yarn run lint.
  • My change has adequate Unit Test coverage.

Terms

Copy link
Collaborator

@claremacrae claremacrae left a comment

Choose a reason for hiding this comment

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

Nice - thank you.

Sorry for the inconvenience, but please could you rebase when you have time,

@claremacrae claremacrae added type: internal Only regards development or contributing scope: rendering of tasks How the plugin displays tasks (except CSS issues) labels Jan 1, 2024
@ilandikov ilandikov force-pushed the refactor-className-to-addClassName branch from 74c905c to ae55f35 Compare January 2, 2024 07:51
@ilandikov
Copy link
Collaborator Author

Sure, done =)

@claremacrae claremacrae merged commit 4894ab2 into obsidian-tasks-group:main Jan 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: rendering of tasks How the plugin displays tasks (except CSS issues) type: internal Only regards development or contributing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants