Skip to content

Conversation

@veronikaslc
Copy link
Contributor

@veronikaslc veronikaslc commented Apr 10, 2025

@veronikaslc veronikaslc force-pushed the mui-upgrade branch 2 times, most recently from ccdc2cb to 5585e85 Compare April 11, 2025 18:36
@veronikaslc veronikaslc force-pushed the minor-dependency-update branch from f336927 to d61a578 Compare April 11, 2025 19:27
@veronikaslc veronikaslc force-pushed the minor-dependency-update branch from d61a578 to 054ab61 Compare April 11, 2025 19:59
@veronikaslc veronikaslc force-pushed the minor-dependency-update branch from 054ab61 to 23c00a8 Compare April 11, 2025 20:37
@veronikaslc veronikaslc force-pushed the minor-dependency-update branch from 23c00a8 to 2ba9a46 Compare April 11, 2025 23:13
@veronikaslc veronikaslc force-pushed the mui-upgrade branch 8 times, most recently from 45d85c2 to a6b4a08 Compare April 15, 2025 05:08
Base automatically changed from minor-dependency-update to dev April 15, 2025 18:10
@veronikaslc veronikaslc marked this pull request as ready for review April 16, 2025 14:55
@veronikaslc veronikaslc added the Test me! Ready for testing label Apr 16, 2025
@marta- marta- requested review from mdpham and sdumitriu April 25, 2025 17:55
Copy link
Member

Choose a reason for hiding this comment

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

In a future PR, since MUI is moving from individual somethingProps to slotProps, we should also change the way we pass component props between our component to something similar, for example here we have textFieldProps, and in AdminResourceListing we have buttonProps.

@veronikaslc
Copy link
Contributor Author

veronikaslc commented Apr 25, 2025

In general as we are developing the app for the users we should prioritise the usability over our personal preferences of any points. As far as I understand the goal, it is not what we personally like or aesthetically prefer is important but what is comfortable for the end user group and brings an effective and productive clear user experience in a first place. If we focus on the most effective workflow of filling the questionnaires, the text presentation is playing the center role here. The spacing, the alignment and decorations all are the equally influetial on user-computer interaction effectiveness.

There was a research conducted to stydy how text alignment is affecting the content disgesting and impacts how the brain processes information visually and cognitively for the production of web pages. This study highlights the importance of prioritizing readability over pure aesthetics in text alignment. The spoiler is that left-aligned body text led to better performance, although participants preferred justified text. Anything text-centered which is more than 3 lines will create additional load of eye movement, brain processing and extra hardness to follow.
That expecially holds for those with reading difficulties or who may use screen readers. Centering can be a barrier for users with disabilities, as it can make it harder for them to scan and read the text.

When centering could be an excellent tool for placing short call-for-attension elements to emphacize, like a link/action button, shourt quote, or navigate, like topic headings, it is not favourable to use centering for listing numerous user instructions or presenting introduction texts. Those are usual common design priciples of web presentations as I see them in current.

@marta-
Copy link
Contributor

marta- commented Apr 25, 2025

In general as we are developing the app for the users we should prioritise the usability over our personal preferences of any points

And that is why I suggested to create a separate task to improve the layout. This is about upgrading a dependency and should remain focused on its purpose.

veronikaslc and others added 3 commits April 25, 2025 14:26
Co-authored-by: Sergiu Dumitriu <[email protected]>
More slotProps applications to previous v.5 props handling way
@veronikaslc
Copy link
Contributor Author

veronikaslc commented Apr 25, 2025

This is about upgrading a dependency and should remain focused on its purpose.

Total support :)

veronikaslc and others added 5 commits April 25, 2025 18:16
Copy link
Member

@sdumitriu sdumitriu left a comment

Choose a reason for hiding this comment

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

I get this error when first displaying some of the tables:

There was a problem inserting the following rule: ".tss-gur5lp-MuiTablePagination-root . MuiTablePagination-select{margin:0 1px;}" DOMException: An invalid or illegal string was specified

I think it's not from our code, but a bug in MuiTable? To trigger it, start adding a new Questionnaire from the dashboard, it shows as soon as the list of questionnaires is loaded, or go to Administration -> Questionnaires, Users, Clinics or other pages with a table.

Comment on lines +4 to +5
['cards-homepage.modelOrganismsIcon']: '@mui/icons-material/Pets'
['cards-homepage.variantsIcon']: '@mui/icons-material/Subtitles'
Copy link
Member

Choose a reason for hiding this comment

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

These 2 aren't even used, they should be removed in a separate PR.

Align top right naviagtion/avatar icon to the right top side;
Removed unused styling class
Align quick result search with the search input width, remove extra padding
Revert changes for QuestionnairePreview Grid
Fixed demo banner and downtime warning icon grid min width
sdumitriu added 2 commits May 2, 2025 12:40
Better alignment for the avatar
Better alignment for the drawer toggle button
@marta-
Copy link
Contributor

marta- commented May 2, 2025

image

sdumitriu and others added 2 commits May 2, 2025 14:24
Improve alignment of the navbar and the content
Removed required classes prop from DateQuestionYear propTypes
@sdumitriu sdumitriu merged commit f2ee8cc into dev May 4, 2025
@sdumitriu sdumitriu deleted the mui-upgrade branch May 4, 2025 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

large PR Test me! Ready for testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants