Skip to content

Code Quality: Introducing StandardViewBase to reduce code duplication #10996

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

Merged
merged 14 commits into from
Jan 23, 2023

Conversation

QuaintMako
Copy link
Contributor

Resolved / Related Issues
Items resolved / related issues by this PR.

What is the purpose of that PR.
A lot of code duplication can be found inside the ViewBrowsers. This PR aims at reducing the duplication by combining the concerned classes inside a standard view.

This preliminary work has for objective to pave the way to a tackle #1928 further down the line.

What has been done

  • Introducing StandardViewBase to gather up the duplicated code in GridViewBrowser, ColumnViewBase and GridViewBrowser.

Validation
How did you test these changes?

  • Built and ran the app.

@QuaintMako
Copy link
Contributor Author

Setting this up as a draft so discussions may ensue to make sure it goes into the right direction.

@yaira2
Copy link
Member

yaira2 commented Jan 13, 2023

@lukeblevins

@yaira2
Copy link
Member

yaira2 commented Jan 13, 2023

@QuaintMako what are your plans for the current BaseLayout, will that be removed?

@QuaintMako
Copy link
Contributor Author

@QuaintMako what are your plans for the current BaseLayout, will that be removed?

What I aim with this PR is to reduce the duplication inside GridViewBrowser, ColumnViewBase and GridViewBrowser who share a pretty close philosophy. They do inherit the BaseLayout, but so does ColumnViewBrowser who is completely different to the others. That's why I introduced something between BaseLayout and the three classes I am refactoring.

For now there is no plan around the BaseLayout. But looking at it, we may have ways to simplify it.

@QuaintMako QuaintMako marked this pull request as ready for review January 22, 2023 15:17
@QuaintMako
Copy link
Contributor Author

Opening up this PR for a first refactoring of the different views.
When it's done and merged, I'll look into defining another object between the Grid and Details layouts to reduce even more the duplication of the code.

yaira2
yaira2 previously approved these changes Jan 23, 2023
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added the ready to merge Pull requests that are approved and ready to merge label Jan 23, 2023
Copy link
Contributor

@ferrariofilippo ferrariofilippo left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 changed the title Code Quality: Introducing StandardViewBase to reduce code duplication. Code Quality: Introducing StandardViewBase to reduce code duplication Jan 23, 2023
@yaira2 yaira2 merged commit 29a65e9 into files-community:main Jan 23, 2023
@QuaintMako QuaintMako deleted the ViewsRefactoring branch February 21, 2023 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants