-
Notifications
You must be signed in to change notification settings - Fork 11
[layouts] Updates PageLayout to be main controller for layouts #309
Conversation
@colleenmcginnis Ready for review 😅 . A few notes
I've found that the implementation of the sidebar title has been inconsistent across sites (some sites use it, others don't). I think that the sidebar title adds repetition to the page as it's often the same name as its respective top nav tav navigation item. If you feel strongly that we should keep it, let me know and I can make it happen. How to reviewIn addition to this PR, you can also:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow @katydecorah. This is incredible!
The PageLayout guide doesn't feel complete yet and think we can and should continue to update it as we roll out. I think we'll be able to better documentation special use cases. I'm curious what you feel is lacking.
I agree! For me the two hardest concepts to wrap my head around (when attempting to unlearn what I already know about our sites) are multi-level (or multi-structured) sites and the difference between navOrder and order (which relies on a common understand of what "top level navigation" means). We should think about adding more examples to the language about these terms. I think we should also consider more consistently linking back to the components tab when we mention specific components in the guides.
If you feel strongly that we should keep it [sidebar titles], let me know and I can make it happen.
I do not feel strongly!
I left some other minor comments and questions below, but the overall approach here is wonderful. I love the separation of structure (in the components) and logic (in the helper functions). ❤️
CHANGELOG.md
Outdated
@@ -4,6 +4,7 @@ | |||
|
|||
- Add `AnalyticsShell` component. [#307](https://github.com/mapbox/dr-ui/pull/307) | |||
- Add Batfish helpers: navigation and topics. | |||
- Update PageLayout to accept layouts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you planning to expand on this as we prep for the rollout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I wanted to wait for a full list of breaking changes.
Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com>
@colleenmcginnis This is really helpful feedback! I'll work to add more about each concept. I'll also look for more linking opportunities. |
* layouts-main: Update package-lock.json Update dependencies, and nvmrc, npm audit fix (#326)
@colleenmcginnis thanks again for your thorough review and notes. Here a few commits to address your specific comments:
Ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👩🍳 😚 Looks great @katydecorah! I made just a few v small suggestions below.
I love the new guide on multi-structured sites. Good timing with the work you're doing with the data services docs. 🥂
Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com>
* layouts-main: [layouts] Updates PageLayout to be main controller for layouts (#309)
* Update CHANGELOG.md * Remove PageLayout testCases from other components (#314) * [layouts] Add AnalyticsShell component (#307) * Add AnalyticsShell component * Update Sentry * Make `env()` function; add tests; use env() in Feedback * Documentation * Merge branch 'layouts-main' into layouts-analytics-shell * layouts-main: Add Changelog to catalog site 0.29.2 Prepare 0.29.2 Update devDependencies, create jest-setup.js (#311) Set peerDependencies to keep frontend libraries in sync (#292) Remove unused highlight.js and util function (#305) Add aria-label to BackToTopButton (#290) * Update package-lock.json * add sentry and web-analytics config tests * Update CHANGELOG.md * [layouts] Add batfish helpers (#310) * Add batfish helpers * use helpers * Enable appending of data * accept `topic` string or `topics` array * Create batfish helpers catalog page * Revert appending topic data * Add live examples * Remove dropdown option (this will be a PageLayout prop) * Update package.json * Update package.json * add links * Update CHANGELOG.md * 🔧 remove markdown from topic descriptions * Revert "🔧 remove markdown from topic descriptions" This reverts commit d41f906. * Set ignore files in babelc config * Revert "Set ignore files in babelc config" This reverts commit 68f143a. * 🔧 Rebuild fixture data from catalog site * Add header-image to test-cases-app * Update batfish.config.js to sync data to fixtures * Exclude helpers tests files from the package; update babel * Reorder tests * Add remark-linters to assert catalog * Add copyeditor to test catalog pages * Update remark-lint-mapbox; add products * Add @mapbox/rehype-prism (for catalog) * join topics and helpers in index file for easier importing * Update package-lock.json * [layouts] Updates PageLayout to be main controller for layouts (#309) * Add layouts to PageLayout comonent * Remove PageLayout from other components (now redundant) * Add examples for each layout * Fix catalog site * New configuration model * Documentation * Allow image paths in cards; enable `fullWidthCards` from frontMatter * Add unProse and noShellHeaderBuffer options * Add BackToTopButton * Wrap ErrorBoundary around contents of PageLayout * documentation * Reorganize; add `customSidebar` prop * Wrap ErrorBoundary around each major section * Update index.md * Remove context functions (will follow up in another PR) * Add tabListAppend prop * Update CHANGELOG.md * move CustomSidebar into sticky * clean up catalog site * Update page-layout.test.js.snap * fix column sizing * fix tabListAppend * add `heading` prop for dynamic headings * add `sidebarTitle` prop * add back append option for buildTopics * update snapshots; fix test * Clear the mobile sticky nav * Better handling of dynamic navTabs; fix ProductMenu homePage url * Pass section tag to navigation * Revert ":wrench: remove markdown from topic descriptions" This reverts commit d41f906. * Add sortArr argument to batfish-helpers/topics; document * add hideCardDescription and hideCardLanguage frontMatter props * Rebuild fixtures/snapshots * Add guides section to catalog * Update fixtures/snapshots * documentation! * Update navigation.test.js.snap * Update fixtures/snapshots * Update fixtures/snapshots * Reduce changes * Clean up * Disable generated file * Update snapshots * Reduce changes * Add products * Documentation * documentation * Update documentation * v1.0.0-layouts.0 * v1.0.0-layouts.1 * Add back missing scroll classes * Remove extra padding * Fix snapshot * Handle single topic in sortingArr * Fix issue where sectioned navigation links were not getting marked active * Documentation * Documentation * Documentation * if topBarSticker is turned off (false), set Stickey topValue to 0 * Apply suggestions from code review Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> * Wrap ProductMenu and PageLayout in backtics * Clean up * Rename basic.js to page.js * add Multi-structured sites guide; make terms consistent (multi-structured) * Rework "Top level navigation" section to "Navigation bar" and expand on the concept * Better distinguish navOrder and order; document order * Clean up * Add links * Update CHANGELOG.md * Apply suggestions from code review Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> * [layouts] Add splitPage Batfish selector (#317) * Add layouts to PageLayout comonent * Remove PageLayout from other components (now redundant) * Add examples for each layout * Fix catalog site * New configuration model * Documentation * Allow image paths in cards; enable `fullWidthCards` from frontMatter * Add unProse and noShellHeaderBuffer options * Add BackToTopButton * Wrap ErrorBoundary around contents of PageLayout * documentation * Reorganize; add `customSidebar` prop * Wrap ErrorBoundary around each major section * Update index.md * Remove context functions (will follow up in another PR) * Add tabListAppend prop * Update CHANGELOG.md * move CustomSidebar into sticky * clean up catalog site * Update page-layout.test.js.snap * fix column sizing * fix tabListAppend * add `heading` prop for dynamic headings * add `sidebarTitle` prop * add back append option for buildTopics * update snapshots; fix test * Add splitPage batfish function * Update snapshots * fix snapshot paths * Add `hideFromNav` option * Exclude batfish 404 page * only set tag for h2 * Clear the mobile sticky nav * Update package-lock.json * Better handling of dynamic navTabs; fix ProductMenu homePage url * Pass section tag to navigation * Revert ":wrench: remove markdown from topic descriptions" This reverts commit d41f906. * Add sortArr argument to batfish-helpers/topics; document * add hideCardDescription and hideCardLanguage frontMatter props * Handle `products` * Update guides * Rebuild fixtures/snapshots * Update fixtures/snapshots * Add guides section to catalog * Update fixtures/snapshots * Documentation * Add tag debugger page * Update fixtures/snapshots * Update fixtures/snapshots * documentation! * Update navigation.test.js.snap * Update fixtures/snapshots * Update fixtures/snapshots * Reduce changes * Update fixtures/snapshots * Apply suggestions from code review Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> * Add SplitPage component; update documentation * Add links to all pages using split-pages * add `why` to redirects section * Add note about heading order * Clean up * Clean up * Add back removed section * Reduce changes * Disable generated file * Update snapshots * Reduce changes * Update fixtures * Add products * Copyedits * copyedits * Documentation * documentation * Update documentation * v1.0.0-layouts.0 * v1.0.0-layouts.1 * Apply suggestions from code review Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> * Merge branch 'layouts-page-layout' into layouts-batfish-sections * layouts-page-layout: v1.0.0-layouts.1 join topics and helpers in index file for easier importing v1.0.0-layouts.0 Update documentation * Tighten up language around partial files and page * Remove products, services, platforms, contenttype for splitPage function (it's covered elsewhere) * Clean up * v1.0.0-layouts.2 * Clean up depenencies * v1.0.0-layouts.3 * Add back missing scroll classes * v1.0.0-layouts.4 * Remove extra padding * v1.0.0-layouts.5 * Fix snapshot * Handle single topic in sortingArr * v1.0.0-layouts.6 * Fix issue where sectioned navigation links were not getting marked active * v1.0.0-layouts.7 * Documentation * Documentation * Documentation * if topBarSticker is turned off (false), set Stickey topValue to 0 * v1.0.0-layouts.8 * [layouts] Get user obj from inside feedback component (#324) * Get user from inside Feedback component * v1.0.0-layouts.9 * Update test cases * Update CHANGELOG.md * [layouts] Fix sidebar height when topbar is not sticky (#325) * If topbar sticker is not sticky, make sidebar go full height * v1.0.0-layouts.10 * Reduce changes * Make sure navTabs has a length before displaying Tablist * v1.0.0-layouts.11 * Sort accordion pages * v1.0.0-layouts.12 * Apply suggestions from code review Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> * Wrap ProductMenu and PageLayout in backtics * Clean up * Rename basic.js to page.js * add Multi-structured sites guide; make terms consistent (multi-structured) * Rework "Top level navigation" section to "Navigation bar" and expand on the concept * Better distinguish navOrder and order; document order * Clean up * Add links * Update CHANGELOG.md * Apply suggestions from code review Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com> * Update packages with security vulnerabilities * Update CHANGELOG.md * v1.0.0-layouts.13 * Hide page sidebar on mobile * v1.0.0-layouts.14 * Always add mt to pages (not using accordion layout) * v1.0.0-layouts.15 * Adjust mt to account for accordion * v1.0.0-layouts.16 * Update CHANGELOG.md * Create mbxMetadata prop and set `window.mbxMetadata` in AnalyticsShell * v1.0.0-layouts.17 * Fix defaultProps * v1.0.0-layouts.18 * Adjust image thumbnail * v1.0.0-layouts.19 * Update mbx-assembly * v1.0.0-layouts.20 * Sort accordion alphabetically then by order * v1.0.0-layouts.21 * Add test case; improve accordion sorting * v1.0.0-layouts.22 * Add icon to accordion * v1.0.0-layouts.23 * Update titleGenerator to not push "Overview" to meta title * v1.0.0-layouts.24 * Add `cardColSize` prop to `CardContainer` to adjust size of cards * Pass props to Search and Feedback components * v1.0.0-layouts.25 * Fix props * Update dependencies * Add passthrough props for Search and Feedback * Clean up catalog * Update deps Co-authored-by: Colleen McGinnis <colleen.mcginnis@mapbox.com>
This branch merges into
layouts-main
.To do:
Topbar/Sticker
and all code to generate the main navigation (ProductMenu
,TabList
,Search
).Feedback
by default with a frontMatter optionhideFeedback: true
to disable it on a given page.[] Add user context:This has been moved to Create Context; Make contextless components contextfull #315 and is now a stretch goal of this sprint.appendTabList
prop to append a custom dropdown to TabList.How to test
QA checklist
6.0.0
.83
since PageLayout test case is duplicating views it's catching ids that are not unique. It's also picking up Improve color contrast of Feedback component #257 and NavigationDropdown, SectionedNavigation - Form elements do not have associated labels #248Open the test cases app locally on: