Skip to content

Achievements Page Mobile View #2868

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 19 commits into from
Mar 28, 2024
Merged

Achievements Page Mobile View #2868

merged 19 commits into from
Mar 28, 2024

Conversation

JovenSoh
Copy link
Contributor

@JovenSoh JovenSoh commented Mar 24, 2024

Description

Closes #2150

Reorganised the achievements page so that its mobile-friendly. The layout has been adjusted to ensure better readability and interaction on smaller screens. Components are now stacked vertically and wrap properly when the screen size does not allow for horizontal alignment.

Before: There was no mobile display for the achievements page and elements where displayed for landscape view.
image

After: Elements now stack vertically with each other when reaching mobile breakpoint.
image

There would be auto scroll for child divs which are larger than their parent divs.
image

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to Test

  1. Navigate to the achievements page on a mobile device or use a browser's mobile view functionality.
  2. Verify that all components are stacked vertically and are responsive to changes in viewport size.

Please provide any additional steps or configurations if applicable.

Checklist

  • I have tested this code on various mobile screen sizes.

@JovenSoh JovenSoh changed the title Mobile achievements Achievements Page Mobile View Mar 24, 2024
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

Please double check the diff and do not commit random changes to the repo, I've pushed some reverts to delete all the miscellaneous files and renamings of various files, I'm just left with one question below:

tsconfig.json Outdated
Copy link
Member

Choose a reason for hiding this comment

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

What's this change for?

Copy link
Contributor Author

@JovenSoh JovenSoh Mar 25, 2024

Choose a reason for hiding this comment

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

I was having errors compiling the frontend code, and could not export sharedb-ace.ts to fix it. However, renaming the files seemed to fix it so I have reverted the change in tsconfig.json.

image image

@RichDom2185
Copy link
Member

Could you provide a screenshot of the UI in the PR description?

@coveralls
Copy link

coveralls commented Mar 24, 2024

Pull Request Test Coverage Report for Build 8463885828

Details

  • 0 of 2 (0.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.006%) to 35.231%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commons/achievement/AchievementManualEditor.tsx 0 1 0.0%
src/pages/achievement/subcomponents/AchievementDashboard.tsx 0 1 0.0%
Totals Coverage Status
Change from base Build 8462889780: -0.006%
Covered Lines: 5119
Relevant Lines: 13567

💛 - Coveralls

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for working on this! I just have the one comment below:

label="View Hidden Achievements"
onChange={() => changeViewHidden(!viewHidden)}
/>
<div className="achievement-manual-editor">
Copy link
Member

Choose a reason for hiding this comment

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

You are wrapping an achievement-manual-editor inside an achievement-manual-editor, is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I have undid this change in a newer commit.

@JovenSoh
Copy link
Contributor Author

Found another issue while testing the Achievements View, more specifically in the control page. The preview was not formatting the achievements correctly, I'm not too sure if this is also the case in production as I do not have admin access.

image image

Have edited so now it would reflect what its supposed to look like in the achievements page.

image image

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@RichDom2185 RichDom2185 merged commit d671afe into master Mar 28, 2024
@RichDom2185 RichDom2185 deleted the mobile-achievements branch March 28, 2024 11:46
izruff pushed a commit that referenced this pull request Apr 2, 2024
* updated for mobile

* fixed formatting

* ran test suites

* minor achievement card changes

* Revert snapshot change

* Remove committed temporary file

* Revert unnecessary changes

* Remove test coverage file

* reverted tsconfig change

* made achievement card scrollable

* Removed additional div

* Add css to achivementcontrol so it would format the page properly

* fixed formatting issues

---------

Co-authored-by: Richard Dominick <34370238+RichDom2185@users.noreply.github.com>
Co-authored-by: sayomaki <sayomayomaki@gmail.com>
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.

Achievements page mobile view
4 participants