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: add a container for FieldLayoutDetails and get data attributes from it #2425

Merged

Conversation

ilandikov
Copy link
Collaborator

Description

Add a container for FieldLayoutDetails and get data attributes from it with a dedicated method.

Motivation and Context

Encapsulate data and behaviour in an error-free way.

Prepare for refactoring of the rendering & layout code to make adding new features easier.

How has this been tested?

Unit tests added for the new class.

Types of 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

@ilandikov ilandikov marked this pull request as draft November 20, 2023 09:02
@claremacrae
Copy link
Collaborator

Thanks. I don't think GitHub lets me know when a PR is marked as ready for review, so do let me know when it is no longer a draft....

@ilandikov
Copy link
Collaborator Author

I thought we discussed that we shall work on this PR tomorrow together. Or do you consider this as good enough to merge and wish start a new branch from main tomorrow? Both are fine for me.

@claremacrae
Copy link
Collaborator

Oh sorry yes, I was unclear...

This is good enough to merge... A definite improvement

@ilandikov ilandikov marked this pull request as ready for review November 20, 2023 09:30
@ilandikov
Copy link
Collaborator Author

ok great, then I mark this as ready for review

@ilandikov
Copy link
Collaborator Author

Please check if the new tests are explicit enough

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.

Thanks. The tests are fine.

@claremacrae claremacrae added type: internal Only regards development or contributing scope: rendering of tasks How the plugin displays tasks (except CSS issues) labels Nov 20, 2023
@claremacrae claremacrae merged commit 983c93a into obsidian-tasks-group:main Nov 20, 2023
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