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

Updated the data view info when selection is applied #7278

Conversation

N-thony
Copy link
Collaborator

@N-thony N-thony commented Mar 7, 2022

Fixes #7277
I have added the info concerning the column selection when it is applied to DataView, I also adjusted the size of the info when the application is minimized or maximized.
@africanmathsinitiative/developers @rdstern this is ready for review. Thanks.

@N-thony N-thony marked this pull request as ready for review March 8, 2022 12:49
@rdstern
Copy link
Collaborator

rdstern commented Mar 19, 2022

I am approving, because it now works. But I wonder whether we should try to do better? Here it is with a select and a filter
image

This is default and they are incredibly small. I am not good on design, but suggest that
a) When a filter if active we say Rows 1 to 108, i.e. we only have the word "Showing" when there is no filter. Similarly for Selects?
b) We drop the word Active in each case as well. So it is just Filter::, and Selection?

@N-thony and @shadrackkibet what is your view?

rdstern
rdstern previously approved these changes Mar 19, 2022
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

Approving, but also see comment with a suggestion.

@shadrackkibet
Copy link
Collaborator

@rdstern I agree.

@N-thony please change.

@N-thony
Copy link
Collaborator Author

N-thony commented Mar 21, 2022

I am approving, because it now works. But I wonder whether we should try to do better? Here it is with a select and a filter image

This is default and they are incredibly small. I am not good on design, but suggest that a) When a filter if active we say Rows 1 to 108, i.e. we only have the word "Showing" when there is no filter. Similarly for Selects? b) We drop the word Active in each case as well. So it is just Filter::, and Selection?

@N-thony and @shadrackkibet what is your view?

@rdstern I made the above change, please test.

@Patowhiz
Copy link
Contributor

Code looks good.
@rdstern @N-thony I wonder why you preferred the column label selection going horizontally.
I'd suggest it to be added vertically. Add this can be done only when there is a column selection. This way we won't need to reduce the font size.

@shadrackkibet shadrackkibet changed the title Updated the dataview info when selection applied Updated the data view info when selection is applied Mar 22, 2022
Copy link
Collaborator

@shadrackkibet shadrackkibet left a comment

Choose a reason for hiding this comment

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

@rdstern if you approve then we can merge.

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@shadrackkibet I am approving, though I quite like @Patowhiz suggestion. If merged as it stands, then perhaps this can be considered later? As you wish.

@shadrackkibet
Copy link
Collaborator

Code looks good. @rdstern @N-thony I wonder why you preferred the column label selection going horizontally. I'd suggest it to be added vertically. Add this can be done only when there is a column selection. This way we won't need to reduce the font size.

@N-thony could you open an issue for this? then you can consider it on a separate PR.

@rdstern another option is to put the label at the top where variable's names are.

@shadrackkibet shadrackkibet merged commit b412756 into IDEMSInternational:master Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the dataview info when column selection is applied
4 participants