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

[#12081] Modify question headers for instructor #12228

Merged
merged 3 commits into from
Mar 23, 2023
Merged

[#12081] Modify question headers for instructor #12228

merged 3 commits into from
Mar 23, 2023

Conversation

jasonqiu212
Copy link
Contributor

Part of #12081
Sub-issue Instructor edit sessions page: modify question headers

Outline of Solution

  • Fixed question number to use <h2>
  • Applied flex-row and flex-col to display in columns when in mobile and in rows when in desktop
  • Fixed question header to use <button> for tab interaction

@zhaojj2209 zhaojj2209 added the s.ToReview The PR is waiting for review(s) label Mar 20, 2023
@zhaojj2209
Copy link
Contributor

InstructorFeedbackEditPageE2ETest is failing, do look into it.

zhaojj2209
zhaojj2209 approved these changes Mar 23, 2023
View reviewed changes
Copy link
Contributor

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

Looks good so far, but text wrap is still a bit iffy on mobile:
Screenshot 2023-03-23 at 22 54 37
Screenshot 2023-03-23 at 22 54 42

A possible fix is to shift the buttons from the right-most column to the bottom-most row on mobile, so that there's more horizontal space for both the text and the buttons. Let's do this in a separate PR though, will create a new card to track this.

@zhaojj2209 zhaojj2209 merged commit 1f618f9 into TEAMMATES:user-friendliness Mar 23, 2023
zhaojj2209 pushed a commit that referenced this pull request Mar 23, 2023
@jasonqiu212 @zhaojj2209
[#12081] Modify question headers for instructor (#12228)
5896af3 
* Modify question headers in instructor edit sessions page

* Fix tests

* Fix e2e tests
zhaojj2209 pushed a commit that referenced this pull request Mar 23, 2023
@jasonqiu212 @zhaojj2209
[#12081] Modify question headers for instructor (#12228)
761513d 
* Modify question headers in instructor edit sessions page

* Fix tests

* Fix e2e tests
zhaojj2209 pushed a commit that referenced this pull request Mar 25, 2023
@jasonqiu212 @zhaojj2209
[#12081] Modify question headers for instructor (#12228)
2456e8e 
* Modify question headers in instructor edit sessions page

* Fix tests

* Fix e2e tests
zhaojj2209 pushed a commit that referenced this pull request Mar 25, 2023
@jasonqiu212 @zhaojj2209
[#12081] Modify question headers for instructor (#12228)
4bba773 
* Modify question headers in instructor edit sessions page

* Fix tests

* Fix e2e tests
@jasonqiu212 jasonqiu212 deleted the 12081-modify-question-headers branch March 26, 2023 18:04
@zhaojj2209 zhaojj2209 self-assigned this Apr 2, 2023
@zhaojj2209 zhaojj2209 added c.Bug Bug/defect report s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels Apr 2, 2023
@zhaojj2209 zhaojj2209 added this to the V8.26.0 milestone Apr 2, 2023
@jasonqiu212 jasonqiu212 mentioned this pull request Sep 3, 2023
Merged
@anniepauline
Copy link

I'd like to work on the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhaojj2209 zhaojj2209 zhaojj2209 approved these changes

@weiquu weiquu Awaiting requested review from weiquu

Assignees

@zhaojj2209 zhaojj2209

Labels
c.Bug Bug/defect report s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V8.26.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jasonqiu212 @zhaojj2209 @anniepauline