Skip to content

space switcher empty spaces #6988

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 2 commits into from
Sep 5, 2022

Conversation

fedrunov
Copy link
Contributor

@fedrunov fedrunov commented Sep 1, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

empty state for new space switching dialog

Motivation and context

closes #6754

Screenshots / GIFs

Light Dark Black
light dark black

@fedrunov fedrunov requested review from a team and ganfra and removed request for a team September 1, 2022 13:15
@fedrunov fedrunov added the Z-NextRelease For issues and PRs which should be included in the NextRelease. label Sep 1, 2022
@fedrunov fedrunov requested review from ericdecanini and bmarty and removed request for ganfra September 5, 2022 10:25
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks, no blocking remarks here.

@@ -71,6 +73,7 @@ class SpaceListFragment :
homeActivitySharedActionViewModel = activityViewModelProvider[HomeSharedActionViewModel::class.java]
roomListSharedActionViewModel = activityViewModelProvider[RoomListSharedActionViewModel::class.java]
views.stateView.contentView = views.groupListView
views.spacesEmptyButton.onClick { onAddSpaceSelected() }
Copy link
Member

Choose a reason for hiding this comment

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

This is maybe not new, but when the user clicks on this button, do we want to close the bottomsheet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericdecanini could you please verify?

Copy link
Contributor

@ericdecanini ericdecanini Sep 5, 2022

Choose a reason for hiding this comment

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

Ideally we don't want to close it immediately in case the user navigates back but we want to close it following a successful space creation.

There's already an issue for this #6950

Edit: This behaviour already works like this in this PR. It also may be that we can close the above mentioned issue if we investigate that this works too in other cases

views.spacesEmptyGroup.isVisible = false
views.groupListView.isVisible = true
}
}
Copy link
Member

Choose a reason for hiding this comment

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

About StateView, the correct code would be:

            is Success -> {
                if (spaces.invoke().isEmpty()) {
                    views.stateView.state = StateView.State.Empty(...)
                } else {
                    views.stateView.state = StateView.State.Content
                }
            }

But maybe there are specific needs here? If this is the spaces_empty_button, maybe we could improve the StateView to integrate it. (can be done later, to not block this.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StateView has exact implementation for Empty state and I hadn't time to extend it to accept custom views (it rises some problems like callbacks for button). So in this case I had to place custom empty state near content view and switch between them. StateView her is mostly for loading state

Copy link
Member

Choose a reason for hiding this comment

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

OK; thanks for explaining.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Can be merged!

@fedrunov fedrunov merged commit b37996e into develop Sep 5, 2022
@fedrunov fedrunov deleted the feature/nfe/app_layout_empty_spaces_list branch September 5, 2022 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-NextRelease For issues and PRs which should be included in the NextRelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Space Switching: Space Bottom Sheet Empty States
3 participants