include searchtips modal #2590
include searchtips modal #2590
Conversation
spec/spec_helper.rb
Outdated
| config.include ViewComponent::TestHelpers, type: :component | ||
| config.include ViewComponent::SystemTestHelpers, type: :component |
There was a problem hiding this comment.
These should go in rails_helper.rb. There are a couple similar configuration entries near the end of the file.
There was a problem hiding this comment.
I will make these changes
There was a problem hiding this comment.
I have made these changes
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
| require 'spec_helper' |
There was a problem hiding this comment.
spec_helper is already included in rails_helper
| require 'spec_helper' |
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
| require 'spec_helper' |
There was a problem hiding this comment.
spec_helper is already included in rails_helper
| require 'spec_helper' |
spec/spec_helper.rb
Outdated
| @@ -11,6 +11,9 @@ | |||
|
|
|||
| FIXTURES_PATH = File.expand_path('fixtures', __dir__) | |||
| require 'simplecov' | |||
| require 'view_component/test_helpers' | |||
| require 'view_component/system_test_helpers' | |||
| require 'capybara/rspec' | |||
There was a problem hiding this comment.
I don't think this is needed
| require 'capybara/rspec' |
spec/spec_helper.rb
Outdated
| require 'view_component/test_helpers' | ||
| require 'view_component/system_test_helpers' |
There was a problem hiding this comment.
These should move to rails_helper with the configs includes below
| // Overrides for search tips link and info icon | ||
| .image-masthead .searchtips-link { | ||
| text-transform: none !important; | ||
| } | ||
|
|
||
| .searchtips-link .bi-info-circle{ | ||
| margin-bottom: 0.15rem; | ||
| } |
There was a problem hiding this comment.
Are these styles needed? The link text looks the same if I remove them and the icon position seems okay.
There was a problem hiding this comment.
The first style applies when we add an image to the masthead. When that happens, there is an existing style that changes the text to uppercase. The style on line 314 prevents that from happening for search tips, since these should stay lowercase even with an image masthead.
For the second style, the vertical spacing did not seem exactly where the Figma screen had it.
There was a problem hiding this comment.
Okay. It's really strange that spotlight upcases the nav links when there's background image, but otherwise doesn't.
| <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-info-circle" viewBox="0 0 16 16"> | ||
| <path d="M8 15A7 7 0 1 1 8 1a7 7 0 0 1 0 14m0 1A8 8 0 1 0 8 0a8 8 0 0 0 0 16"/> | ||
| <path d="m8.93 6.588-2.29.287-.082.38.45.083c.294.07.352.176.288.469l-.738 3.468c-.194.897.105 1.319.808 1.319.545 0 1.178-.252 1.465-.598l.088-.416c-.2.176-.492.246-.686.246-.275 0-.375-.193-.304-.533zM9 4.5a1 1 0 1 1-2 0 1 1 0 0 1 2 0"/> | ||
| </svg></span><%= t('search_tips.title') %></a> |
There was a problem hiding this comment.
There's an info icon component already in spotlight that could be used via blacklight_icon('info'). Should we use that icon or override it locally with this one?
There was a problem hiding this comment.
Happy to use an existing one. I'll try that out, thanks.
There was a problem hiding this comment.
I set one up using the blacklight icon method. This also changed some styling (there's an extra space that the component adds within the blacklight icon next to the svg element)
|
Requires waiting for approval of projectblacklight/spotlight#3171 with some additional changes in exhibits |
…icon for info button, styling
…ts in top level directory
|
Screenshots of current view in local development:
|
| .blacklight-icons:hover { | ||
| text-decoration: none; | ||
| } |
There was a problem hiding this comment.
Is this doing anything? I don't see any underlined whitespace when I remove this.
There was a problem hiding this comment.
It was displaying an underline earlier but now no longer is (perhaps due to upstream changes in Blacklight?) I've removed this line since it no longer appears necessary.
Closes #2487
What this PR does:
Also, some notes:
I asked @alundgard and it is ok if the close button has a red outline for now. Also, the mobile view shows the search tips button on top of the search bar and not the bottom (which is what the designs show). This is also ok for now.