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

Edit site description directly from /sites/<id> #2701

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

williamjallen
Copy link
Collaborator

This PR follows #2680 and moves the "edit site description" functionality to /sites/<id>, finishing the last piece necessary to remove the legacy site management page.

Before editing:
image

While editing:
image

After saving (no page reload):
image

@josephsnyder
Copy link
Member

@williamjallen, functionally it seems pretty solid but I have a few questions

  • Should we limit the description editing to someone who has claimed the site?

  • Related to above, is there a way to drop history without database access? If someone comes in and writes unwanted text, I don't want to see that in the history of the site until it scrolls off the bottom of the webpage.

  • There seems to be a limit of characters in the text box, but there is no indication that such a limit exists. Can you add some text to say, "Max of #### characters" ?

  • Could whitespace be preserved in the display of the description, like it is in the database? I started drawing a little ASCII table and it shows in the edit and in the database but not when it has saved:
    image
    image
    vs
    image

@williamjallen
Copy link
Collaborator Author

@josephsnyder

  • Should we limit the description editing to someone who has claimed the site?
  • Related to above, is there a way to drop history without database access? If someone comes in and writes unwanted text, I don't want to see that in the history of the site until it scrolls off the bottom of the webpage.

Any user can claim any site, so tying the description editing to claiming a site (basically, subscribing to notifications and having your name shown in the maintainers list) does not really prevent anyone from doing anything malicious. I think the best way to control this is to control the users who have access to a given CDash system. In the future, it might make sense to have "verified" users who can do things like setting the site description as well as "unverified" users who have read-only access.

  • There seems to be a limit of characters in the text box, but there is no indication that such a limit exists. Can you add some text to say, "Max of #### characters" ?

The description field is arbitrarily limited to 255 characters in the database. Since the description is set by users while the system information is set at submission time, it doesn't really make sense to link these two things together. As such, I opened #2672 to track splitting these two things apart. I plan to remove the character limit when that happens. Additionally, I plan to add information about the user who made the change to partially address the issues you brought up in your first point. I like the idea of putting a brief message alerting users about the current character limit though.

  • Could whitespace be preserved in the display of the description, like it is in the database? I started drawing a little ASCII table and it shows in the edit and in the database but not when it has saved

Hmm, I'm not convinced that preserving whitespace is desirable here. While the idea of drawing ASCII figures is nice, I'm doubtful that many people will want to do it. If we preserve whitespace, the wrapping behavior will have to change to instead add scrollbars if the description field is too small. I think this is meant for a brief description or maybe links to associated resources. Given the small width of the history column, I don't think putting preformatted text there makes sense.

Copy link
Member

@josephsnyder josephsnyder left a comment

Choose a reason for hiding this comment

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

Thanks for the feedback. I like the max character addition as I much prefer to be stuck typing rather than losing data on save. LGTM!

@williamjallen williamjallen added this pull request to the merge queue Feb 6, 2025
Merged via the queue into Kitware:master with commit 813cd4c Feb 6, 2025
7 checks passed
@williamjallen williamjallen deleted the edit-site-description branch February 6, 2025 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants