Update Breadcrumbs/Add Activity Stream UI #9083
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
@qe we should add a visual test here #3786 (comment) |
|
Build succeeded.
|
|
Hey @jlmitch5 this looks awesome, I've found so far:
|
jakemcdermott
approved these changes
Jan 19, 2021
|
Thanks for the review @tiagodread !
Fixed
Fixed
I've reset the page to 1 when the activity stream type changes
We talked about this offline. It's due to the fact type doesn't accept the __icontains suffix operator. |
|
Build failed.
|
|
@tiagodread fixed all the breadcrumb issues you mentioned. Thanks again for such a thorough review!
Fixed
Fixed
Fixed
Fixed (and I added a min-height to the title section, so even if it's not rendered yet, it's render want cause updown flashing in any case)
Fixed |
|
Build succeeded.
|
|
Build failed.
|
|
Build succeeded.
|
|
@jlmitch5 @marshmalien added #9122 to track the settings work |
added 3 commits
January 21, 2021 10:01
- show last breadcrum item as Title on new line - add activity stream type (to display activity stream icon link in header)
added 13 commits
January 21, 2021 10:01
… in activity streram
team roles title crumb missing various inventory crums missing make it so inventories and templates don't get rid of data needed to generate the crumb config
…tes and aid in testing
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded (gate pipeline).
|
This was referenced Feb 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is broken up into 3 main commits. Some detail on each:
TODO:
"update Breadcrumb component to ScreenHeader"
Relevant issues: #7727, #8465
Part of the requirements for the Activity Stream UI (that is pulled over from the legacy UI), is that each view (aside from Managment Jobs), has an activity stream link which pre-filters the view to the specific type of resource(s) that page shows. So for example, when you are on the Organizations page and you click the Activity Stream link in the top-right header, it should take you to the Activity Stream route and only show you Organization-based activity (creating, editing, deleting, etc.)
Because we already had a component that rendered the header across the app in a consistent way (
<Breadcrumbs />, I decided to extend that component (with an additional prop for the stream type to filter), and rename it to<ScreenHeader />in order for the name to better encapsulate its new responsibilities.While making these changes, I decided to tackle an issue that was raised a few weeks ago...Patternfly has stopped rendering the
BreadcrumbHeadingcomponent (always our last breadcrumb) in a similar way to theTitlecomponent. I talked with taufique, and he said we still want this functionality--projects are moving to a system where this functionality is implemented by rolling your own solution withTitlefor the last breadcrumb. Basically, I just duplicated the recursive Crumb component, and only render the final step (asTitle). I also get rid of theBreadcrumbsbar (and the unnecessary top padding it creates) when there is only one crumb.For testing, due to the way this utilizes the
useRouteMatchhook, the solution I settled on was to utilize the actual React Router lib in the relevant files as they would fail if I didn't (open to other approaches on this point if anyone has any better ideas)."don't strip out non-namespaced params when encoding url search params"
When designing the qs libs, we made sure all lists had a namespace with which we could add to the params, so that when interacting with various UI elements which update the list (such as search, sort and pagination/page size setting), any other search params that were hanging around would not be stripped out. while adding the params was done correctly, it seemed like the encoding side of things was not. I updated the qs lib's function for encoding the params into the url param to accept an addtional "other search params" argument, and in all the places where it is called, I get those other params so they can be passed.
This was necessary because the activity stream type needed to be a search param, but it kept getting stripped out and set back to "all activity".
"add activity stream ui"
Relevant Issues:
?actor__usernamesearch column: Allow search for actions performed by user in activity stream #7230This commit contains all the new activity stream UI code (api model, route, list, etc.). It uses the new react-table component.
It is virtually identical to the old UI, with a few small differences which are:
string +=). It also does not render html strings (which is against our new CSP), but rather JSX components.