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

Split off the Ground Truth page #3853

Merged
merged 11 commits into from
Feb 26, 2025
Merged

Split off the Ground Truth page #3853

merged 11 commits into from
Feb 26, 2025

Conversation

chrisvanrun
Copy link
Contributor

@chrisvanrun chrisvanrun commented Feb 25, 2025

Part of the pitch:

A quick pre-lude:

  • split the ground truth from being a tab in the the reader study detail to its own view
  • add a warning when the reader study is not configured for educational purposes

Other than above, I did not change the content of the page.

Considered to refactor all reader study views to have the configuration side-panel, much like the algorithms. But have put that out of scope (for now).

Showcase

The warning is no longer there.

afbeelding

@jmsmkn
Copy link
Member

jmsmkn commented Feb 25, 2025

Considered to refactor all reader study views to have the configuration side-panel, much like the algorithms. But have put that out of scope (for now).

It is not the case for algorithms, we do not have such a feature anywhere. Please do not do large refactoring like that without discussion in a pitch first.

@chrisvanrun
Copy link
Contributor Author

chrisvanrun commented Feb 25, 2025

It is not the case for algorithms, we do not have such a feature anywhere. Please do not do large refactoring like that without discussion in a pitch first.

Definitively! I thought I should mention this here as it would be a sensible comment to have on a split-from-tab PR.

I thought algorithms implemented it already but it actually are the Challenge admin pages (i.e. phase settings, general config, et cetera) that do that.

@chrisvanrun chrisvanrun requested a review from jmsmkn February 25, 2025 16:48
@chrisvanrun
Copy link
Contributor Author

Given the changes were out of style with the other apps and it was suggested to update them, I've taken the oppertunity to update the style of permissions and route param typing within the reader study app.

Copy link
Member

@jmsmkn jmsmkn 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!

@jmsmkn jmsmkn merged commit 8fc5c23 into main Feb 26, 2025
8 checks passed
@jmsmkn jmsmkn deleted the split-ground-truth-page branch February 26, 2025 08:18
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.

2 participants