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

Remove Coming Soon Pre publish Modal CSS #40

Merged
merged 3 commits into from
Aug 30, 2024
Merged

Conversation

mr-vara
Copy link
Contributor

@mr-vara mr-vara commented Aug 28, 2024

Proposed changes

This PR removes unwanted CSS from Pre Publish Modal and uses existing classes instead.

Type of Change

  • New feature (non-breaking change which adds functionality)

Video

Screen.Recording.2024-08-28.at.11.17.09.AM.mov

Checklist

  • I have read the CONTRIBUTING doc
  • I have viewed my change in a web-browser
  • Linting and tests pass locally with my changes

Further comments

Without External CSS:
image

With External CSS:
image

Not able to achieve exactly without using External CSS. We may have more similar looking modal with inline CSS, but it might not look good from code perspective. I'm not able to add the Launch icon, as it is disturbing the alignment of the buttons. Seems like icon buttons needs to be in a flex container. If we use flex, we need to use a media query for small screen sizes. For the other missing similarities like shadow/border, right alignment et., we need to use inline CSS.

Comment
People
Assignee:
varaprasad.maruboina
Vara Prasad Maruboina
Reporter:
emullins
Evan Mullins
Votes: 0 Vote for this issue
Watchers: 2
Stop watching this issue
Dates
Created: 01/Mar/24 5:48 PM
Updated: 1 minute ago
Automated Log Work
Your timer: Not startedA
Development
Create branch
Agile
View on Board

@mr-vara mr-vara marked this pull request as draft August 28, 2024 05:53
@mr-vara mr-vara marked this pull request as ready for review August 29, 2024 06:48
@mr-vara mr-vara requested a review from circlecube August 29, 2024 06:49
@circlecube
Copy link
Member

This is good. The icon isn't that important and this looks integrated with WordPress styles. I'm fine simplifying the code and files needed in place of the icon, @chrisdavidmiles would you agree?

Looking at the setComingSoon method though, this component should use the NewfoldRuntime.comingSoon API rather than creating its own via the settings endpoint. We can then remove the utils js files too. Also removed the icon files since they're not used anymore and then threw in a few inline styles just to get the look closer to what it was earlier.

Some inline styles:
Screenshot 2024-08-29 at 2 49 23 PM

@circlecube circlecube requested a review from wpalani August 29, 2024 19:04
@circlecube
Copy link
Member

Tagging @wpalani to review since I've got code in here.

update build files for new version
@circlecube circlecube merged commit cde7aba into main Aug 30, 2024
4 of 6 checks passed
@circlecube circlecube deleted the feature/press0/1959 branch August 30, 2024 20:32
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.

3 participants