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

Dm 3592 anchor tags #542

Merged
merged 12 commits into from
Oct 7, 2022
Merged

Conversation

bradjohnson92008
Copy link
Contributor

@bradjohnson92008 bradjohnson92008 commented Sep 21, 2022

JIRA issue link

https://agile6.atlassian.net/browse/DM-3592

Description - what does this code do?

Testing done - how did you test it/steps on how can another person can test it

Create anchor # links to jump to a particular element/component on the same page or when a page loads.

Note to Devs: PB show page /app/views/page/show.html.erb must have an element (<div, ..etc) with an id that is generated dynamically for each instance of the component. For example if there are 5 accordion components on the same page a counter can be used to generate unique IDs for each accordion component - i.e.,

  <% when 'PageAccordionComponent' %>
    <% accordion_ctr += 1 %>
  <div id="accordion_anchor_<%= accordion_ctr %>"/>

So the unique id for each accordion component is "accordion_anchor_1", "accordion_anchor_2", "accordion_anchor_3" ... and so on. See page/show.html.erb for code above. This same technique can be used to create anchor (jump links) for other types of components as well. THIS is already setup for the accordion component so for testing you do not have to do this.

Note: If creating a link to an anchor tag on the same page.. you can either just add the anchor tag as the url i.e., "#accordion_anchor_3" or provide the path and the anchor to the internal url i.e., "/brads-page-group/brads-new-test-page#accordion_anchor_link_3". The first method will open the page in a new tab and scroll to the anchor. The second method - entering the full internal url will not open a new tab - and just scroll to the anchor tag.


TEST

  1. Create or Edit an existing PB page and ensure there are at least one accordion component on the page and the component or components are not at the top of the page.
  2. Create or Edit a different page and create a link (either in WYSIWIG or other link type component) and set the link to navigate to the page that has the accordion component(s). At the end of the link just add the anchor tag to the end of the url and specify which accordion component to jump to. i.e., #accordion_anchor_1, #accordion_anchor_2 ...etc. i.e., /contests/shark-tank#accordion_anchor_2".

image

When the user clicks on the link to the accordion page the new page will load and automatically scroll to the anchor element defined in the url.. ie., #accordion_anchor_2 image

image

image

Link to mock-ups/mock ups (image file if you have it) (if applicable)

N/A

Acceptance criteria

  1. Investigate Brad’s HTML method
  2. User can link to FAQ section on About page
  3. Determine way to do this as one off
  4. If possible, determine way to do this that can be replicated in Page Builder
  5. Nice to have: Specific FAQ is open

Definition of done

  • Unit tests written (if applicable)
  • e2e/accessibility tests written (if applicable)
  • Events are logged appropriately
  • Documentation has been updated, if applicable
  • A link has been provided to the originating JIRA issue
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

add fix for anchor tag/links in chrome.
build out anchor tags for all Accordions/(FAQs) on a page.
test to ensure accordion component displays
spec - add click event to display accordion text.
spec for accordion
Copy link
Contributor

@JoshingYou1 JoshingYou1 left a comment

Choose a reason for hiding this comment

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

Nice work on this Brad! I left a few comments for ya.

@@ -56,6 +55,8 @@

<%# Accordion %>
<% when 'PageAccordionComponent' %>
<% accordion_ctr += 1 %>
<div id="accordion_anchor_<%= accordion_ctr %>"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add a div here? Can't we just use the container div on the next line? Also, would it make sense to add a column in the DB called anchor_link_name, or something like that, which would allow the user to create their own anchor name, and then we can just .parameterize it? I'd like to know your thoughts on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I see what you mean.. add the column (anchor_link_name) to the page_components table right? So when we save the page in PB we would just create a guid for the anchor_link_name and then embed that in the PB show page? Only problem I see with that it may be harder for the content creator to create the anchor links in PB instead of just using a convention like "accordion_anchor_1"..etc or "map_anchor_1" ...etc. ? Lets talk through this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved anchor_tag id to div container instead of above div container.. that works.

it 'Should display the accordion component' do
expect(page).to have_content('FAQ 1')
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test that checks to make sure that when the anchor link name is added to the url, the page jumps to the correct top position?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked in to this... and found https://stackoverflow.com/questions/18948687/capybara-how-to-test-link-to-different-section-of-same-page .. so not sure if this is possible.

}, 300);
}
});

Copy link
Contributor

Choose a reason for hiding this comment

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

What does this code do? Removing it didn't seem to affect the desired outcome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for a work around in chrome... yeah I wasn't expecting to have to add any js but found that the old school anchor links don't work in chrome without this js code in place to find the the anchor. Were you using chrome or safari?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was using chrome, and it was working correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can test without it. This is what I was running in to.. https://stackoverflow.com/questions/51887886/anchor-links-not-working-on-chrome but this may only affect certain versions of chrome.. or may be a chrome/windows thing.. not sure. My version of chrome is 106.0.5249.61.

moved anchor tag to div container instead of above div container.
Add test for multiple accordions and anchor tags.
test
Copy link
Collaborator

@camillevilla camillevilla left a comment

Choose a reason for hiding this comment

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

This works great for me, Brad!

I have a slight future proofing suggestion: what if instead of using accordion_ctr, you interpolated the component.id into <div id="accordion_anchor_? This might make it easier to implement something like having the anchor link open the specific FAQ in a future iteration. It would also keep the link persistent if content creators rearrange the order of the FAQs.

spec/features/pages/show_page_spec.rb Outdated Show resolved Hide resolved
app/assets/javascripts/_page_show.es6 Outdated Show resolved Hide resolved
PR fixes for anchor tags.
@bradjohnson92008
Copy link
Contributor Author

Hey @camillevilla - I like the way you are thinking on this. I thought about using a unique ID instead of a counter but since the Content Creators will be inputting that ID in the link in PB - how would they know what that ID was? Unless they knew how to inspect the element.. and I didn't want to go there. lol. But your right if they change the order of the elements (FAQs or whatever in the future) the content creator would have to update the links with the correct ID based on it's new order on the page.

@camillevilla
Copy link
Collaborator

@bradjohnson92008 that's a good point. A future iteration is probably going to need some UX input. If we do a future iteration of this, we might use something like the heading links in the Redux documentation:
anchor-link-hover

@bradjohnson92008 bradjohnson92008 merged commit 0b12f6b into feature/communities Oct 7, 2022
@bradjohnson92008 bradjohnson92008 deleted the dm-3592-anchor-tags branch October 7, 2022 17:38
JoshingYou1 added a commit that referenced this pull request Oct 21, 2022
* Dm 3533 (#543)

* DM-3533: Added new component and component image models

* DM-3533: Started 'Add Image' functionality

* DM-3533: Removed TinyMCE code that was messing with WYSIWYG links

* DM-3533: Fixed TinyMCE issues

* DM-3533: Added more fields and added delete functionality to component images

* DM-3533: Finished all backend functionality

* DM-3533: Added some front end code for compound body component

* DM-3533: Added configurable bottom and top margin. Cleaned up some code

* DM-3533: Added admin panel tests for CompoundBodyComponent

* DM-3533: Added a bunch of tests

* DM-3533: Added unit tests for new models

* DM-3533: Cleaned up some code

* DM-3533: Fixed some failing tests

* DM-3533: Fixed failing tests

* DM-3533: Refactored some code and added another test

* DM-3533: Reverted data type for paperclip attachments

* DM-3533: Added to Pages show page and a few tests

* DM-3533: Added mobile styling and another test

* DM-3533: Cleaned up some code

* DM-3533: Commented out selenium_chrome line from spec_helper

* DM-3533: Added outline to images that belong to a CompoundBodyComponent

* DM-3533: Removed unnecessary 'validate' call

* DM-3533: Added external link styling to WYSIWYG links for compound body component

* DM-3533: Removed some unnecessary code

* DM-3533: Cleaned up some JS code

* DM-3533: Fixed issue with external link styling for PageComponentImages

* DM-3533: Added ':thumb' style to PageComponentImage on Page show page

* Dm 3592 anchor tags (#542)

* dm-3592

add fix for anchor tag/links in chrome.

* dm-3592

build out anchor tags for all Accordions/(FAQs) on a page.

* dm-3592

test to ensure accordion component displays

* dm-3592

spec - add click event to display accordion text.

* dm-3592

spec for accordion

* dm-3592

* dm-3508

moved anchor tag to div container instead of above div container.

* dm-3592

Add test for multiple accordions and anchor tags.

* dm-3508

test

* dm-3508

* dm-3508

PR fixes for anchor tags.

* Dm 3529 (#546)

* DM-3533: Added new component and component image models

* DM-3533: Started 'Add Image' functionality

* DM-3533: Removed TinyMCE code that was messing with WYSIWYG links

* DM-3533: Fixed TinyMCE issues

* DM-3533: Added more fields and added delete functionality to component images

* DM-3533: Finished all backend functionality

* DM-3533: Added some front end code for compound body component

* DM-3533: Added configurable bottom and top margin. Cleaned up some code

* DM-3533: Added admin panel tests for CompoundBodyComponent

* DM-3533: Added a bunch of tests

* DM-3533: Added unit tests for new models

* DM-3533: Cleaned up some code

* DM-3533: Fixed some failing tests

* DM-3533: Fixed failing tests

* DM-3533: Refactored some code and added another test

* DM-3533: Reverted data type for paperclip attachments

* DM-3533: Added to Pages show page and a few tests

* DM-3533: Added mobile styling and another test

* DM-3533: Cleaned up some code

* DM-3533: Commented out selenium_chrome line from spec_helper

* DM-3533: Added outline to images that belong to a CompoundBodyComponent

* DM-3533: Removed unnecessary 'validate' call

* DM-3533: Added external link styling to WYSIWYG links for compound body component

* DM-3533: Removed some unnecessary code

* DM-3533: Cleaned up some JS code

* DM-3529: Added image and image alt text fields to page

* DM-3533: Added ability to add image to a Page in the admin panel

* DM-3533: Fixed issue with external link styling for PageComponentImages

* DM-3533: Added ':thumb' style to PageComponentImage on Page show page

* DM-3529: Working on frontend for header image

* DM-3529: Added mobile styles

* DM-3529: Adjusted styles based on design's feedback

* DM-3529: Finished writing tests and cleaning up styling

* DM-3529: Improved a test

* DM-3529: Fixed failing tests

* DM-3529: Fixed up the schema and uncommented headless chrome

* DM-3529: Removed redundant 'private' call and cleaned up a little bit of code

* DM-3534 DM-3561: Add Events and News Components (#544)

Co-authored-by: Camille Villa <[email protected]>

* Dm 3508 map component (#541)

* dm-3508

Adding support/dependencies for map component.

* dm-3508

Page Builder UI

* dm-3508

* dm-3508

documenting page_map_components schema

* dm-3508

backend and infrastructure for front end.

* dm-3508

* dm-3508

bus logic: fetching diffusion histories for selected practices.

* dm-3508

* dm-3508

center map and zoom

* dm-3508

styles, zoom level

* dm-3508

Fix query for adopting facilities

* dm-3508

removed dbg

* dm-3508

removed dbgs

* dm-3508

marker infowindow

* dm-3508

info window

* dm-3508

logic for statuses

* dm-3508

front end stuff

* dm-3508

info window styling text changes.

* dm-3508

border radius..

* dm-3508

map zoom

* dm-3508

border padding for info window card style

* dm-3508

use padding shorthand

* dm-3508

add margin between description and map bottom.

* dm-3508

info window static size.

* dm-3508

infowindow styles...

* dm-3508

Added in Abbr Short_name for info window content.. i.e., 23 "XR" innovations.. for a specific Facility.

* dm-3508

margin above num innovations in infowindow

* dm-3508

schema change

* dm-3508

added test.

* dm-3508

* dm-3508

fixed issue with text wrapping and ignoring padding.

* dm-3508

info window styles

* dm-3508

make map component mobile friendly.

* dm-3508

more info window and border styles for map component info window.

* dm-3508

* dm-3508

* dm-3508

info window styles

* dm-3508

addressing testing and PR comments.

* dm-3508

refactored some code to use ActiveRecord vs arrays.

* dm-3508

Refactored some code in the controller.

* dm-3508

renamed infowindow partial to _page_map_infowindow

* dm-3508

another slight refactor...

* dm-3508

UPdate based on PR comments.

* dm-3508

Fixed styling for Info Window Facility Link.

* dm-3508

Map and info window styles.. enums for Diffusion History statuses.. etc.

* dm-3508

Addend links for innovations within the map component markers info-window

* dm-3508

forgot to update these changes in schema.rb

* dm-3508

remove commented code and fix margin in info window.

* dm-3508

Added model test.

* dm-3508

more tests for map

* dm-3508

try to fix test.

* dm-3508

Fix test.

* dm-3508

* dm-3508

ok.. lets try different syntax..

* m-3508

display-none?

* dm-3508

forgot the div

* dm-3508

* dm-3508

* dm-3508

* dm-3508

info window styles

* dm-3508

* dm-3508

info window size and positioning

* dm-3508

* dm-3508

* dm-3508

Style changes per design.

* dm-3508

spec for clicking map marker and displaying infowindow

* dm-3508

err in spec.

* dm-3508

Admin Panel test page for creating a map component

* dm-3508

removed debuggers

* dm-3508

Admin panel spec

* dm-3508

Update IDs to match db change for "display_successful_adoptions" for spec.

* Revert "dm-3508"

This reverts commit 8e352bd

* dm-3508

re-factor spec

* dm-3508

more refactor for specs

* dm-3508

* dm-3508

info window spec

* dm-3508

setting ids back to be consistent in map_page_comonent_form.html.arb

* dm-3508

find the map marker...

* dm-3508

Remove other community features from schema that are in other component branches.

* dm-3508

PR updates.

* dm-3508

PR updates

* dm-3508

pr suggestions

* dm-3508

moved helper method to model - refactored method.

* dm-3508

* dm-3508

test not working now...

* dm-3508

PR Comments

* dm-3508

InfoWindow refactors

* dm-3508

Update/Refactor short_name column to map_info_window_text

* dm-3508

updated page_map_component.short_name field to map_info_window_text.

* dm-3508

Update tests for map due to column name change.

* dm-3508

update spec for page_map_component field name change.

* dm-3508

pr comments.

* dm-3508

INfo Window styles.

* dm-3508

Decrease maxWidth and maxHeight

* dm-3508

removed commented code from spec.

* dm-3508

adjustedpixel offset for new maxWidth, maxHeight.

* dm-3508

Fix indent for comment headers.

* dm-3508

revert back to "if" instead of "unless"

* Removed duplicate code in admin/pages.rb

Co-authored-by: Joshua Drumm <[email protected]>

* Community Feature

replace total adoption count with just count of "adopted practices within the community"

* Changed '.count' to '.length' for practice_data array

* DM-3626: Refactored grid column setup for 'CompoundBody' component (#549)

* Dm compound body spacing fix (#550)

* Renamed 'margin' columns to 'padding' and refactored the code to reflect those changes

* Reverted some schema changes

* Fixed a failing test

* Fixed a couple of typos

* DM 3631: Support /communities URL for Community pages (#551)

Co-authored-by: Camille Villa <[email protected]>
Co-authored-by: Joshua Drumm <[email protected]>

* DM-3627: Adjusted styles to align with DM homepage styles (#553)

* Dm 3632 (#552)

* dm-3632

Added XR Network dropdown nav in header.

* dm-3632

highlighting/bolding for currently selected menu item

* dm-3632

add some tests for nav.

* dm-3632

update path info based on community/xr-network routes

* dm-3632

PR comments adjustments

* dm-3632

test for top nav communities.

* dm-3632

test for home page.

* dm-3632

test for communities about page.

* dm-3632

* dm-3632

* dm-3632

* dm-3632

* dm-3632

revert tests - not working.

* dm-3632

this test should work..

* dm-3632

slug needs to be 'home'

* dm-3632

change slug back to home.

* dm-3632

Adding path tests for all xr-network pages.

* dm-3632

adding the pages.

* Communities redirect fix (#555)

* Fixed community redirect issue and added a test

* Improved test scenario description

* Trying to fix test on CI

* Hopefully fixed test on CI

Co-authored-by: Brad Johnson <[email protected]>
Co-authored-by: Camille Villa <[email protected]>
Co-authored-by: Camille Villa <[email protected]>
Co-authored-by: bradjohnson92008 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants