Skip to content

Fix highlighting of the New tab after Bootstrap 5 upgrade#1328

Merged
osma merged 1 commit intomasterfrom
fix-tabs-after-bootstrap-5
Jun 2, 2022
Merged

Fix highlighting of the New tab after Bootstrap 5 upgrade#1328
osma merged 1 commit intomasterfrom
fix-tabs-after-bootstrap-5

Conversation

@osma
Copy link
Member

@osma osma commented Jun 2, 2022

Reasons for creating this PR

The New tab wasn't properly highlighted after changes made in the Bootstrap 5 upgrade (#1182). Note that the "New and deprecated" tab variant was working OK.

Before:

image

After:

image

Link to relevant issue(s), if any

Description of the changes in this PR

  • make sure the "active" CSS class is properly applied for the New tab
  • move some variable declarations to a more proper place in the twig templates

Known problems or uncertainties in this PR

The template code for rendering the tabs is duplicated in five(!) different Twig templates: changes.twig, group-contents.twig, group-index.twig, light.twig and vocab.twig. This should really be refactored and implemented in just one place. I will open a separate issue about this.

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@codecov
Copy link

codecov bot commented Jun 2, 2022 •
edited
Loading

Codecov Report

Merging #1328 (7971ff6) into master (290296c) will increase coverage by 0.08%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1328      +/-   ##
============================================
+ Coverage     70.59%   70.68%   +0.08%     
  Complexity     1646     1646              
============================================
  Files            32       32              
  Lines          3744     3786      +42     
============================================
+ Hits           2643     2676      +33     
- Misses         1101     1110       +9
Impacted Files Coverage Δ
model/Concept.php 83.63% <0.00%> (-0.22%) ⬇️
controller/RestController.php 22.38% <0.00%> (-0.01%) ⬇️
model/ConceptMappingPropertyValue.php 82.82% <0.00%> (+0.17%) ⬆️
model/sparql/GenericSparql.php 91.27% <0.00%> (+0.20%) ⬆️
model/sparql/JenaTextSparql.php 90.00% <0.00%> (+0.25%) ⬆️
model/resolver/WDQSResource.php 92.85% <0.00%> (+0.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 290296c...7971ff6. Read the comment docs.

@osma osma self-assigned this Jun 2, 2022
@osma osma added the bug label Jun 2, 2022
@osma osma added this to the 2.15 milestone Jun 2, 2022
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
50.0% 50.0% Duplication

kinow
kinow approved these changes Jun 2, 2022
View reviewed changes
Copy link
Collaborator

@kinow kinow left a comment

Choose a reason for hiding this comment

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

CI happy, screenshot and code look good! Thanks (again 🙇 ) @osma !

@osma osma mentioned this pull request Jun 2, 2022
Closed
@osma
Copy link
Member Author

osma commented Jun 2, 2022

The template code for rendering the tabs is duplicated in five(!) different Twig templates: changes.twig, group-contents.twig, group-index.twig, light.twig and vocab.twig. This should really be refactored and implemented in just one place. I will open a separate issue about this.

I opened issue #1329 about this.

@osma osma merged commit 1c4fd32 into master Jun 2, 2022
@osma osma deleted the fix-tabs-after-bootstrap-5 branch June 2, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@kinow kinow kinow approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@osma osma

Labels

bug

Projects

None yet

Milestone

2.15

Development

Successfully merging this pull request may close these issues.

2 participants

@osma @kinow