Skip to content

Comments

[5.1] Button and link text color issue on focus#41749

Closed
sandewt wants to merge 2 commits intojoomla:5.1-devfrom
sandewt:patch-8
Closed

[5.1] Button and link text color issue on focus#41749
sandewt wants to merge 2 commits intojoomla:5.1-devfrom
sandewt:patch-8

Conversation

@sandewt
Copy link
Contributor

@sandewt sandewt commented Sep 14, 2023

Text is not readable on focus

Pull Request for Issue #40306, #41153 and #41172

Summary of Changes

The Pull Request #40435 for issue #40306, caused the issues #41153 and #41172.
Hopefully this PR will provide the overall solution. Or there is possible a better css solution.

Testing Instructions

Build: npm ci
Clear the browser cache (historie) after the patch if necessary

Actual result BEFORE applying this Pull Request

The button and the link text is NOT readable on focus.

Expected result AFTER applying this Pull Request

The button and the link text is always readable on focus.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Thanks @RickR2H

Text is not readable on focus
@RickR2H
Copy link
Member

RickR2H commented Sep 15, 2023

@sandewt on reviewing the code I would suggest leaving this code as is. The problem is now only in the header and the footer.

I would suggest changing the following:

build\media_source\templates\site\cassiopeia\scss\blocks_layout.scss on line: 19

a:not(.btn) {
  color: currentColor;

  &:hover,
  &:focus {
    color: var(--gray-200);
  }
}

build\media_source\templates\site\cassiopeia\scss\blocks_footer.scss on line: 15

a:not(.btn) {
  color: currentColor;

  &:hover,
  &:focus {
    color: var(--gray-200);
  }
}

For testing put the following code in a custom html module in the header, in the footer and in an article:

<a href="#" class="btn btn-primary">Primary</a>
<a href="#" class="btn btn-secondary">Secondary</a>
<a href="#" class="btn btn-success">Success</a>
<a href="#" class="btn btn-danger">Danger</a>
<a href="#" class="btn btn-warning">Warning</a>
<a href="#" class="btn btn-info">Info</a>
<a href="#" class="btn btn-light">Light</a>
<a href="#" class="btn btn-dark">Dark</a>
<a href="#" class="btn btn-link">Link</a>
<a href="#">Default Link</a>

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Sep 20, 2023
@ceford
Copy link
Contributor

ceford commented Sep 22, 2023

I have tested this item ✅ successfully on c305129

Please note that I tested this on Joomla 5.0.0-beta3-dev. And a better command is npm run build:css


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

@ceford
Copy link
Contributor

ceford commented Sep 22, 2023

Also tested on 4.3.5-dev works there too, except the Banner test Get started button is white on green both with and without the patch.


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

@RickR2H
Copy link
Member

RickR2H commented Sep 22, 2023

@sandewt please consider my solutions as stated in: #41749 (comment)

@ceford please create 3 modules. In the banner, footer and the main-top with the code for all the buttons.

<a href="#" class="btn btn-primary">Primary</a>
<a href="#" class="btn btn-secondary">Secondary</a>
<a href="#" class="btn btn-success">Success</a>
<a href="#" class="btn btn-danger">Danger</a>
<a href="#" class="btn btn-warning">Warning</a>
<a href="#" class="btn btn-info">Info</a>
<a href="#" class="btn btn-light">Light</a>
<a href="#" class="btn btn-dark">Dark</a>
<a href="#" class="btn btn-link">Link</a>
<a href="#">Default Link</a>

@ceford
Copy link
Contributor

ceford commented Sep 23, 2023

Tested again on 4.3.5-dev and 5.0.0-beta3-dev. I see no difference between the two. The patch fixes the problem with Default links in the footer but not the links with the btn btn-link style. In the banner and main-top positions the buttons and link all appear normal. But in the footer the Secondary and Light buttons have invisible text except on hover/focus. Firefox on Mac.


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

@sandewt
Copy link
Contributor Author

sandewt commented Sep 23, 2023

But in the footer the Secondary and Light buttons have invisible text except on hover/focus.

@ceford I can confirm this, see the image. Would you please mark your test as unsuccessful?

errors_issue

[EDIT Even without this patch the image is the same]

@ceford
Copy link
Contributor

ceford commented Sep 23, 2023

I have not tested this item.

I think the footer button problems make my successful test invalid.


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

@sandewt
Copy link
Contributor Author

sandewt commented Sep 23, 2023

I think the footer button problems make my successful test invalid.

At first I thought this too.
But I just ran a test. It turns out that this issue is not caused by pr #40435. In fact, this is a new issue.

@sandewt
Copy link
Contributor Author

sandewt commented Sep 23, 2023

The problem is now only in the header and the footer.

Not correct, also in an acticle

See:#40420

@Fedik Fedik added the bug label Sep 24, 2023
@ceford
Copy link
Contributor

ceford commented Sep 24, 2023

I have tested this item ✅ successfully on c305129

This PR fixes the problem with links where the background is dark - the Default Link in the data set suggested by @RickR2H - so it a Success.

The links styled as buttons is a separate issue - I confirm the problem in positions below-top and footer but not in an article with a white background. So I assume there will be a separate PR to deal with that.


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

@RickR2H
Copy link
Member

RickR2H commented Sep 28, 2023

I have tested this item ✅ successfully on c305129

@sandewt Could you make an new PR to fix the colors in the header and the footer with the changes mentioned by me?


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

@joomla-cms-bot joomla-cms-bot removed Updates Requested Indicates that this pull request needs an update from the author and should not be tested. NPM Resource Changed This Pull Request can't be tested by Patchtester bug PR-4.3-dev labels Sep 28, 2023
@RickR2H
Copy link
Member

RickR2H commented Sep 28, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 28, 2023
@joomla-cms-bot joomla-cms-bot added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Sep 28, 2023
@HLeithner HLeithner changed the base branch from 4.3-dev to 5.1-dev September 30, 2023 22:48
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@richard67 richard67 changed the title [4.3] Button and link text color issue on focus [5.1] Button and link text color issue on focus Oct 3, 2023
@ceford
Copy link
Contributor

ceford commented Oct 19, 2023

Comment in the Forum that this was not fixed in 4.4!


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

@sandewt
Copy link
Contributor Author

sandewt commented Nov 25, 2023

@sandewt Could you make an new PR to fix the colors in the header and the footer with the changes mentioned by me?

Comment in the Forum that this was not fixed in 4.4!

@RickR2H Thanks for your suggestion. I think others are more skilled at writing the most appropriate code. Partly due to the fact that the PR code was originally intended for J4.4. And there may also be a better solution for this PR.

@RickR2H
Copy link
Member

RickR2H commented Nov 27, 2023

@sandewt Maybe better close this ticket and I'll create a new one with all the fixes mentioning this PR.

@sandewt sandewt closed this Nov 27, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 27, 2023
@sandewt
Copy link
Contributor Author

sandewt commented Nov 27, 2023

@sandewt Maybe better close this ticket and I'll create a new one with all the fixes mentioning this PR.

That seems like a good idea to me. Closed.

@sandewt
Copy link
Contributor Author

sandewt commented Nov 27, 2023

Thanks all

@sandewt
Copy link
Contributor Author

sandewt commented Dec 17, 2023

Please test #42504

@ceford thanks in advance

@sandewt sandewt deleted the patch-8 branch August 19, 2024 13:51
@sandewt sandewt restored the patch-8 branch August 19, 2024 13:51
@sandewt sandewt deleted the patch-8 branch August 19, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants