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

Use a row for the bread crumb (as in other visual elements nearby) and include padding #964

Merged
merged 1 commit into from
Apr 1, 2020

Conversation

kinow
Copy link
Collaborator

@kinow kinow commented Mar 31, 2020

Use a <div class="row"> around the breadcrumb as in other elements to force it to occupy the whole row-area.

Would be easier if the elements were flex. Haven't used bootstrap in a while, but looks like it supports it too in the version 4.x.

Closes #963 but needs a bit more of testing before it's ready for review. (tested with docker-compose.yml, and found nothing broken in the UI).

Applying changes locally in firefox development mode it looks as follows:

image

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #964 into master will decrease coverage by 7.55%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #964      +/-   ##
============================================
- Coverage     66.36%   58.80%   -7.56%     
- Complexity     1504     1505       +1     
============================================
  Files            32       32              
  Lines          3728     4207     +479     
============================================
  Hits           2474     2474              
- Misses         1254     1733     +479     
Impacted Files Coverage Δ Complexity Δ
model/resolver/WDQSResource.php 51.51% <0.00%> (-42.93%) 3.00% <0.00%> (ø%)
model/sparql/GenericSparql.php 68.17% <0.00%> (-24.35%) 311.00% <0.00%> (ø%)
model/sparql/JenaTextSparql.php 75.00% <0.00%> (-16.84%) 15.00% <0.00%> (ø%)
model/ConceptMappingPropertyValue.php 73.63% <0.00%> (-9.02%) 39.00% <0.00%> (ø%)
model/resolver/Resolver.php 75.00% <0.00%> (-6.82%) 6.00% <0.00%> (ø%)
model/resolver/LinkedDataResource.php 86.66% <0.00%> (-6.20%) 3.00% <0.00%> (ø%)
model/GlobalConfig.php 87.15% <0.00%> (-4.20%) 51.00% <0.00%> (ø%)
model/ConceptProperty.php 95.83% <0.00%> (-4.17%) 17.00% <0.00%> (ø%)
model/SkosmosTurtleParser.php 54.83% <0.00%> (-3.79%) 18.00% <0.00%> (ø%)
model/Model.php 81.52% <0.00%> (-3.42%) 105.00% <0.00%> (ø%)
... and 12 more

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 45456a0...5ea7391. Read the comment docs.

@kinow kinow marked this pull request as ready for review April 1, 2020 00:10
@osma
Copy link
Member

osma commented Apr 1, 2020

Wow thanks @kinow ! We were just discussing how to approach this but you got there first with your fix :)

@osma osma merged commit 17361d5 into NatLibFi:master Apr 1, 2020
@osma
Copy link
Member

osma commented Apr 1, 2020

Tested it locally, seems to work fine.

@osma osma added the bug label Apr 1, 2020
@osma osma added this to the 2.5 milestone Apr 1, 2020
@kinow
Copy link
Collaborator Author

kinow commented Apr 1, 2020

Wow thanks @kinow ! We were just discussing how to approach this but you got there first with your fix :)

Thanks for testing so quickly. Hopefully will fix the issue for the user that reported it (dev.finto appears to be working, but not sure if it was deployed already)

@osma
Copy link
Member

osma commented Apr 1, 2020

Yes the fix is now deployed in dev.finto.fi, as it follows the master branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad layout of preferred term
2 participants