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

Adds Suggested Tag in Explore Space #5826

Merged
merged 4 commits into from Apr 27, 2022

Conversation

ericdecanini
Copy link
Contributor

@ericdecanini ericdecanini commented Apr 22, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

(Could be feature because it's purely an addition, or could be bugfix because it's techinically a fix for a regression from several versions ago. Considering it a bugfix for that reason)

Content

Adds Suggested Tag in Explore Space (for suggested rooms)

Motivation and context

Closes #5715

Many versions ago (though I am unsure which), we had a suggested tag on each suggested room in the Explore Space screen. This was before the Open and Join buttons were added in the same screen, which is when I assume we lost this suggested tag.

This PR is adding this back in, with a slight design change from what it used to be to accomodate for the new Open and Join buttons.

Screenshots / GIFs

Before After
Light lightbefore lightafter
Dark darkbefore darkafter

Tests

  • Join (or be in) a space with suggested rooms (or make one yourself)
  • Go to Explore Space of that space
  • See suggested tag under suggested rooms

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 12

Checklist

@ericdecanini ericdecanini added PR-Small PR with less than 20 updated lines X-Needs-Design May require input from the design team labels Apr 22, 2022
@ericdecanini ericdecanini requested review from niquewoodhouse, a team and onurays and removed request for a team April 22, 2022 19:54
@github-actions
Copy link

github-actions bot commented Apr 22, 2022

Unit Test Results

114 files  ±0  114 suites  ±0   1m 29s ⏱️ +4s
202 tests ±0  202 ✔️ ±0  0 💤 ±0  0 ±0 
678 runs  ±0  678 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 039c9c6. ± Comparison against base commit 2839d14.

♻️ This comment has been updated with latest results.

@niquewoodhouse
Copy link

@ericdecanini can I just check please, are the margins around the items missing or is it just the screenshot being cropped in on the sides? Thanks

@ericdecanini
Copy link
Contributor Author

image

@niquewoodhouse Screenshot cropping indeed. The margins should've been untouched.

@@ -145,6 +145,7 @@ class SpaceDirectoryController @Inject constructor(
matrixItem(matrixItem)
avatarRenderer(host.avatarRenderer)
topic(info.topic)
suggested(info.suggested ?: false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we generally use info.suggested.orFalse().

Copy link
Contributor

@onurays onurays left a comment

Choose a reason for hiding this comment

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

Nice improvement, LGTM!

@niquewoodhouse
Copy link

@niquewoodhouse Screenshot cropping indeed. The margins should've been untouched.

Cool thanks, I just wanted to check to make sure!

@ericdecanini ericdecanini merged commit 5ac0555 into develop Apr 27, 2022
@ericdecanini ericdecanini deleted the bugfix/eric/missing-suggested-tag branch April 27, 2022 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Small PR with less than 20 updated lines X-Needs-Design May require input from the design team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggested rooms are no longer marked in explore rooms
3 participants