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

Fix modals style and general preferences #2377

Merged

Conversation

wadjih-bencheikh18
Copy link
Member

closes: #1771

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (309cdd8) 52.05% compared to head (209571c) 52.05%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2377   +/-   ##
=======================================
  Coverage   52.05%   52.05%           
=======================================
  Files          52       52           
  Lines        2553     2553           
  Branches       84       84           
=======================================
  Hits         1329     1329           
  Misses       1222     1222           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

The goal of react-science is to take care of the style of common components as much as possible. We should do general changes to the style of modals in react-science so it benefits other projects and doesn't make nmrium more complex.
Also I think in this case, the styles for the body of the about dialog (color of links, bullet points, size of image, etc) is very specific and should not be applied to other modals.
Can you please open a PR on react-science with a subset of the changes that we can review?

…ces-dialog-box-should-be-centered-vertically
@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 23, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 209571c
Status: ✅  Deploy successful!
Preview URL: https://a414c898.nmrium.pages.dev
Branch Preview URL: https://1771-general-preferences-dia.nmrium.pages.dev

View logs

@wadjih-bencheikh18
Copy link
Member Author

Can you please open a PR on react-science with a subset of the changes that we can review?

Here's the PR that will allow us to change header text position in react-science modal
zakodium-oss/react-science#512

@targos
Copy link
Member

targos commented Jun 27, 2023

What is the modal for which you want to change the header position?

@wadjih-bencheikh18
Copy link
Member Author

wadjih-bencheikh18 commented Jun 30, 2023

What is the modal for which you want to change the header position?

I have to change the position of general preferences and about us modals and it's already fixed

@targos
Copy link
Member

targos commented Jun 30, 2023

I have trouble to understand what this PR does. I see it adds "maxWidth" property, but there is no difference between this PR and https://dev.nmrium.org (main branch), whatever is the size of the browser window.

@wadjih-bencheikh18
Copy link
Member Author

I have trouble to understand what this PR does. I see it adds "maxWidth" property, but there is no difference between this PR and https://dev.nmrium.org (main branch), whatever is the size of the browser window.

I followed what Daniel mentioned here #1771 (comment) to check if there any difference. Now i'm trying to change modal content style so it will adapt screen size

@targos targos enabled auto-merge (squash) July 4, 2023 07:41
@targos targos merged commit 7d114d9 into main Jul 4, 2023
11 of 12 checks passed
@targos targos deleted the 1771-general-preferences-dialog-box-should-be-centered-vertically branch July 4, 2023 08:20
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.

General preferences dialog box should be centered vertically
2 participants