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

Programming exercises: Improve details page layout #9208

Conversation

florian-glombik
Copy link
Contributor

@florian-glombik florian-glombik commented Aug 12, 2024

Checklist

General

Client

  • I strictly followed the client coding and design guidelines.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

Motivation and Context

Unify the layout of the programming exercise details- and edit-page by displaying information in the details page in the same sections as on the edit page.
Reducing the scope of some sections by introducing the new section Build Details, which is displayed directly after the General section, making the repository, checkout directory and Repository Diff more prominent on the details page.

Description

  • Moved the short name, auxiliary repositories, and recreate build plans to a new section Build Details that is added as second section
  • Moved the selection of categories up in the edit mode (within the General section) to be aligned with the details page

Steps for Testing

Prerequisites:

  • 1 Instructor
  1. View the details page of a programming exercise
  2. See that a new section Build Status is introduced
  3. Go to the create/edit page of a programming exercise
  4. See that the section Build Status is reflected in the create/edit page
  5. Verify that creating / editing / importing exercises still works

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Screenshots

Change Before After
Added Build Details Section in Details View image image
Added Build Details Section in Create/Edit/Import View image image

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced a method for displaying detailed build configuration information for programming exercises.
    • Enhanced interaction by integrating a build details component, providing real-time validation feedback during the update process.
    • Improved organization and clarity of the programming exercise detail presentation, enhancing user experience.
    • Added a user-friendly interface for managing build configurations and auxiliary repositories.
  • Bug Fixes

    • Resolved issues with form validity tracking across multiple sections of the programming exercise update interface.
  • Chores

    • Removed auxiliary repository management components to streamline the user interface.
    • Added a new build details component to improve functionality and user insights.

@florian-glombik florian-glombik added client Pull requests that update TypeScript code. (Added Automatically!) component:Programming labels Aug 12, 2024
@florian-glombik florian-glombik added this to the 7.5.1 milestone Aug 12, 2024
@florian-glombik florian-glombik self-assigned this Aug 12, 2024
@github-actions github-actions bot added the tests label Aug 19, 2024
@florian-glombik florian-glombik temporarily deployed to artemis-test2.artemis.cit.tum.de August 19, 2024 12:37 — with GitHub Actions Inactive
@florian-glombik florian-glombik marked this pull request as ready for review August 19, 2024 13:17
@florian-glombik florian-glombik requested a review from a team as a code owner August 19, 2024 13:17
Copy link
Contributor

@az108 az108 left a comment

Choose a reason for hiding this comment

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

reapprove after small changes 👍 LGTM

Copy link
Contributor

@Jan-Thurner Jan-Thurner left a comment

Choose a reason for hiding this comment

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

reapprove

Copy link
Contributor

@SimonEntholzer SimonEntholzer left a comment

Choose a reason for hiding this comment

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

reapprove

Copy link
Contributor

@JohannesWt JohannesWt left a comment

Choose a reason for hiding this comment

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

Reapprove code after fixing client test

Copy link
Member

@krusche krusche left a comment

Choose a reason for hiding this comment

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

I think the changes are not well thought out due to the following reasons

  1. The language section includes many build details, in particular docker image and build script. User might be confused why those can be found in the language section. However, they heavily depend on the chosen programming language, therefore it makes sense to first choose the language and then edit those settings.
image
  1. It was kind of nice to have 5 rather distinct sections like in other exercises. Adding a 6th one somehow breaks this and might lead to issues when looking at the page on mobile devices (did you test this?)

  2. We have carefully chosen all section headlines to consist of one word, now we have one with two words "Build Details".

I am open to improve the programming exercise sections, but it must really make sense and should not neglect previous discussion and design ideas!

I suggest we discuss again how to split all settings into 5 sections in a way that editing becomes easy.

This change should also be aligned with the new simple mode that I would like to see for beginners that basically leaves out all the more advanced options.

Just to repeat this from a previous Slack discussion, the simple mode could leave out the following in my opinion:

  • Channel Name
  • Short Name (just make sure it's unique)
  • Participation (individual would be default)
  • Allow Offline IDE und allow Online Editor
  • Mode could then potentially be combined with General in simple mode and only contains the Difficulty
  • Project Type and With exemplary dependency
  • Enable Static Code Analysis
  • Sequential Test Runs
  • Customize Build Script
  • Submission Policy
  • Timeline simplified: not Start Date, no Example Solution Publication Date, no Run Tests after Due Date
  • all options under assessment
  • Presentation
  • Plagiarism Control
  • Competencies

@florian-glombik
Copy link
Contributor Author

florian-glombik commented Sep 1, 2024

Thank you for the detailed feedback!

  1. The name of the new section does not appear to be well-chosen, I agree that it is confusing to have a section Build Details and mainly configuring the build in the Language section. As we are actually editing the repositories here (defining the repository names via the short name and optionally adding auxiliary repositories), Repositories might be a better name for the section and prevent confusion?

    The described problem of the interrelation between the sections Language and Repositories is something we should discuss for the auxiliary repositories, as the chosen language might have an impact on the configuration of the checkout directories (which is the case for the previous and the new solution), for this advanced case it would be more intuitive to define the language and package name first. In other cases, where no checkout directories are manually defined, I am not aware of differences for the user in the configuration based on the language selection.

  2. I was not aware that having 5 sections was an active design decision. While the number of sections would be aligned with text-, modeling- and file-upload-exercises I am not sure if unifying the number of sections prevails the clarity in the programming exercise overview, as currently the Language section can be quite long in the exercise details view, and we might be able to allow for a quicker and more intuitive access to chosen configurations by deriving another section, which is the goal of this PR (together with displaying the Review Changes/Template-Solution-Diff feature at a more prominent place in the details section).

    I did not test the responsiveness beforehand, as I relied on the extended form-status-bar to handle width changes properly. I did now test it with the following result:

    If the horizontal scrollbar for 767px - 790px widths is an issue, we might be able to resolve this by improving the responsiveness of the form-status-bar or adjusting the breakpoint in this case. For phones, I would argue that it doesn't make a reasonable difference.

  3. Ramona mentioned that the section titles have been intentionally chosen to be only one word. I misunderstood that this is the case to ensure that the section titles are short and easy to memorize. I was not aware that a single word for the section title is a fixed requirement.

Thanks for pointing out the previous thoughts that would have been violated! I am sorry about neglecting previous ideas with this PR, which was not my intention. I was not aware of 2 and only touched 3 in a discussion with Ramona, after which I thought Build Details would be a reasonable section title.

Follow-up Questions

For my understanding the reduced configuration options of the Simple Mode should be reflected in the details view, which would result in the Language section being reduced anyways (Sequential Test Runs, Build Script, and Docker Image would not be displayed in the details view). Do we have the same understanding?

If we decide to limit ourselves to 5 sections, I do think that the sections Mode, Problem, and Grading are well-defined and blend in nicely with the other exercise types. This would leave the open question if we want to move the definition of the Short Name and Auxiliary Repositories to the Language section, due to the dependence on the Programming Language and Package Name selection.

  • pro
    • might be more logical when editing
    • more consistent with the details view, where the repositories are already listed under the language section
  • contra
    • makes the configuration of the Language section longer in advanced mode

I would be in favor of moving the short name and auxiliary repository selection to the language section, as I would rate the pro bullet-points as more important. However, I have no strong opinion on this topic.

@krusche krusche modified the milestones: 7.5.1, 7.5.2 Sep 2, 2024
@florian-glombik florian-glombik removed this from the 7.5.2 milestone Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) component:Programming tests
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

8 participants