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

Feature/sc 28027/re factor searchresultslist #1943

Conversation

stevekaplan123
Copy link
Contributor

In SearchResultList, the prop types are no longer an array but rather a string that is either 'text' or 'sheet'. Many state variables in SearchResultList were dictionaries with keys 'text' and 'sheet' and so were refactored as well. There is also no longer a prop tab and SearchTabs and updateTab have been removed. In SearchPage and SearchResultList, textSearchState and sheetSearchState are collapsed into one searchState but I did not do this throughout the app as I felt this PR was already big enough. I created a new story to address collapsing textSearchState and sheetSearchState into searchState throughout the app.

textSearchState={this.state.textSearchState}
sheetSearchState={this.state.sheetSearchState}
type={this.state.searchType}
searchState={this.state[`${this.state.searchType}SearchState`]}
Copy link
Contributor

Choose a reason for hiding this comment

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

fix tabbing on this line

Copy link
Contributor

Choose a reason for hiding this comment

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

This is great. Good job deleting unused code and overall simplifying things quite a bit.

@stevekaplan123 stevekaplan123 removed the request for review from edamboritz August 5, 2024 08:59
@stevekaplan123 stevekaplan123 merged commit 5653de6 into modularization-main Aug 5, 2024
16 of 18 checks passed
@stevekaplan123 stevekaplan123 deleted the feature/sc-28027/re-factor-searchresultslist branch August 5, 2024 13:17
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.

2 participants