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

[5.0] Dark Mode: MFA button #42085

Merged
merged 2 commits into from Oct 9, 2023
Merged

[5.0] Dark Mode: MFA button #42085

merged 2 commits into from Oct 9, 2023

Conversation

Quy
Copy link
Contributor

@Quy Quy commented Oct 7, 2023

Partial Pull Request for Issue #41794.

Summary of Changes

Don't use outline button in order to be visible in dark mode.

Actual result BEFORE applying this Pull Request

dark-mfa

Expected result AFTER applying this Pull Request

41794-mfa

@Quy Quy added the Dark Mode label Oct 7, 2023
@HLeithner HLeithner added this to the Joomla! 5.0.0 milestone Oct 7, 2023
@HLeithner
Copy link
Member

@wilsonge please review and we need a test

@brianteeman
Copy link
Contributor

Don't know if it matters or not but this changes the button to solid from an outline button in light and dark mode

@Quy
Copy link
Contributor Author

Quy commented Oct 7, 2023

It should be fine as it has the same style as here:

add-note

@richard67 richard67 changed the title [5.0] Dark Mode: MTA button [5.0] Dark Mode: MFA button Oct 7, 2023
@wilsonge
Copy link
Contributor

wilsonge commented Oct 7, 2023

We probably do need to fix the outline color to match what the main button is. However it also doesn’t hurt to unify the styling across core (if we aren’t using the outline class elsewhere)

@HLeithner
Copy link
Member

rc2 is coming soon, which marks the end of non critical PRs.

@Hackwar
Copy link
Member

Hackwar commented Oct 8, 2023

I have tested this item ✅ successfully on dae9e0f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42085.

@chmst
Copy link
Contributor

chmst commented Oct 8, 2023

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42085.

@chmst
Copy link
Contributor

chmst commented Oct 8, 2023

I have tested this item ✅ successfully on dae9e0f


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42085.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla! 5.0.0 milestone Oct 8, 2023
@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42085.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 8, 2023
@richard67 richard67 added this to the Joomla! 5.0.0 milestone Oct 8, 2023
@bembelimen
Copy link
Contributor

I agree with @wilsonge that the overall outline button definition should be fixed (happens hopefully with #42010), as this PR fixes the issue, I'll merge it.

@bembelimen bembelimen merged commit 58695c3 into joomla:5.0-dev Oct 9, 2023
4 of 5 checks passed
@bembelimen
Copy link
Contributor

Thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 9, 2023
@Quy Quy deleted the 41794-mfa branch October 9, 2023 13:53
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.

None yet

9 participants