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.

Sign up for GitHub

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

Jump to bottom

fix: Column Toggle Select Element Positioning #754

Merged
merged 2 commits into from
Oct 30, 2023
Merged

fix: Column Toggle Select Element Positioning #754

merged 2 commits into from
Oct 30, 2023

Conversation

KevinWu098
Copy link
Member

Summary

The Select element was positioned to a visibility: 'hidden' FormControl, not the IconButton as it should have. This creates an off-centered visual experience.

Before:

After:
Screenshot 2023-10-21 at 7 35 03 PM

Test Plan

  1. This change is primarily visual in nature, so confirm that the positioning is correct on mobile and on desktop
  2. Additionally, there was some bugginess while adding the Popover and Select, so make sure that selecting and deselecting elements doesn't reposition the element (it shouldn't)

Issues

Closes #753

@github-actions github-actions bot requested a review from MinhxNguyen7 October 22, 2023 02:39
ap0nia
ap0nia previously requested changes Oct 27, 2023
View reviewed changes
Copy link
Collaborator

@ap0nia ap0nia left a comment

Choose a reason for hiding this comment

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

Epic work 👏 Just some minor adjustments and we should be good to go.

apps/antalmanac/src/components/RightPane/CoursePane/CoursePaneButtonRow.tsx Outdated Show resolved Hide resolved
apps/antalmanac/src/components/RightPane/CoursePane/CoursePaneButtonRow.tsx Show resolved Hide resolved
apps/antalmanac/src/components/RightPane/CoursePane/CoursePaneButtonRow.tsx Show resolved Hide resolved
apps/antalmanac/src/components/RightPane/CoursePane/CoursePaneButtonRow.tsx Show resolved Hide resolved
@KevinWu098 KevinWu098 requested a review from ap0nia October 27, 2023 23:51
MinhxNguyen7
MinhxNguyen7 approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@MinhxNguyen7 MinhxNguyen7 left a comment

Choose a reason for hiding this comment

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

LGTM!

@KevinWu098 KevinWu098 dismissed ap0nia’s stale review October 30, 2023 18:59

Can't implement (my skill issue)

@KevinWu098 KevinWu098 merged commit c0c8742 into main Oct 30, 2023
6 checks passed
@ap0nia ap0nia deleted the fix-column-toggle-button branch November 8, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MinhxNguyen7 MinhxNguyen7 MinhxNguyen7 approved these changes

@ap0nia ap0nia Awaiting requested review from ap0nia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Column Toggle Select is Positioned Wrong
3 participants
@KevinWu098 @MinhxNguyen7 @ap0nia