NPM Resource Changed bug PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar hans2103
hans2103
30 Jun 2024

Pull Request for Issue #43708 .

Summary of Changes

This PR will adjust the styling of the alert boxes. Overrides are done using css custom properties instead of overriding css attributes.

Testing Instructions

I've used the following html to test alert boxes

<div class="alert" role="alert">
    class="alert"<br/>
    A simple primary alert—check it out! <a href="#" class="btn btn-primary">button</a>
</div>
<div class="alert alert-primary" role="alert">
    class="alert alert-primary"<br/>
    A simple primary alert—check it out! <a href="#" class="btn btn-primary">button</a>
</div>
<div class="alert alert-secondary" role="alert">
    class="alert alert-secondary"<br/>
    A simple secondary alert—check it out! <a href="#" class="btn btn-secondary">button</a>
</div>
<div class="alert alert-success" role="alert">
    class="alert alert-success"<br/>
    A simple success alert—check it out! <a href="#" class="btn btn-success">button</a>
</div>
<div class="alert alert-danger" role="alert">
    class="alert alert-danger"<br/>
    A simple danger alert—check it out! <a href="#" class="btn btn-danger">button</a>
</div>
<div class="alert alert-warning" role="alert">
    class="alert alert-warning"<br/>
    A simple warning alert—check it out! <a href="#" class="btn btn-warning">button</a>
</div>
<div class="alert alert-info" role="alert">
    class="alert alert-info"<br/>
    A simple info alert—check it out! <a href="#" class="btn btn-info">button</a>
</div>
<div class="alert alert-light" role="alert">
    class="alert alert-light"<br/>
    A simple light alert—check it out! <a href="#" class="btn btn-light">button</a>
</div>
<div class="alert alert-dark" role="alert">
    class="alert alert-dark"<br/>
    A simple dark alert—check it out! <a href="#" class="btn btn-dark">button</a>
</div>

Actual result BEFORE applying this Pull Request

Screenshot 2024-06-30 at 20 49 28 Screenshot 2024-06-30 at 20 49 18

Expected result AFTER applying this Pull Request

Screenshot 2024-06-30 at 20 30 24 Screenshot 2024-06-30 at 20 30 13

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

avatar hans2103 hans2103 - open - 30 Jun 2024
avatar hans2103 hans2103 - change - 30 Jun 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Jun 2024
Category Repository NPM Change
avatar brianteeman
brianteeman - comment - 30 Jun 2024

Why are you overriding the native bootstrap alert classes. It is doing this sort of thing that causes problems in the first place,

avatar hans2103
hans2103 - comment - 1 Jul 2024

Why are you overriding the native bootstrap alert classes. It is doing this sort of thing that causes problems in the first place,

I do agree with you. I am going to refactor .alert and joomlaalert.
Less overrides means less maintenance.

I convert this PR as draft

avatar HLeithner
HLeithner - comment - 2 Sep 2024

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

avatar HLeithner HLeithner - change - 2 Sep 2024
Title
[5.1] #43708 Alert links system container - a11y
[5.2] #43708 Alert links system container - a11y
avatar HLeithner HLeithner - edited - 2 Sep 2024
avatar Hackwar
Hackwar - comment - 22 Nov 2024

@hans2103 are you still working on this? If you want this to go into 5.2 still, please finish the PR and lift the draft status, so that we can get people to test this and hopefully be merged into the next 5.2 release.

avatar hans2103
hans2103 - comment - 22 Nov 2024

@Hackwar @brianteeman refactor the joomla-alert as I wrote in my comment implies a PR on https://github.com/joomla-projects/custom-elements/tree/c1f118a5c96f382a43b735b46eb7e04e888987ed/src

A PR on that repo will not make it to 5.2.x I guess... :-)
Hope to get some time this weekend to fix current colors in joomla/joomla-cms repo and refactor the joomla-alert another time

avatar hans2103 hans2103 - change - 22 Nov 2024
Labels Added: NPM Resource Changed bug PR-5.2-dev
avatar hans2103
hans2103 - comment - 22 Nov 2024

@Hackwar I leave the PR as is without further changes.
I do agree with @brianteeman to embrace Bootstrap implementation of alert. This requires a change to https://github.com/joomla-projects/custom-elements/tree/c1f118a5c96f382a43b735b46eb7e04e888987ed/src which is out of scope of this PR.

avatar Hackwar
Hackwar - comment - 22 Nov 2024

The longer I'm looking at this, the more I think this would be a PR for 5.3. It isn't exactly a broken feature, but an improvement, so I would suggest to put this into 5.3 and there you can definitely also update the custom-elements package. Would that be okay for you?

avatar hans2103
hans2103 - comment - 23 Nov 2024

@Hackwar indeed... I am not fixing code over here, just improving it by removing overrides and using custom properties

Add a Comment

Login with GitHub to post a comment