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

User favorites #3255

Merged
merged 156 commits into from
Feb 15, 2019
Merged

User favorites #3255

merged 156 commits into from
Feb 15, 2019

Conversation

KlapTrap
Copy link
Contributor

@KlapTrap KlapTrap commented Dec 3, 2018

Be able to favorite all of the CF entity types and show them on a new "home page dashboard" page.

This work should be able to be used by any extension and should allow for currently unknown entity types.

The aim of this work is to give users a area where they can see entities they care about in one place (from any level within CF) without having to navigate through the CF hierarchy every time.

@cfdreddbot
Copy link

✅ Hey KlapTrap! The commit authors and yourself have already signed the CLA.

@KlapTrap KlapTrap self-assigned this Dec 3, 2018
Copy link
Contributor

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

Home Page - Favourites

  • It looks like you don't have any favorites message needs converting to
  • CF Summary card links to Deploy Application when cf is disconnected
  • Icons needed for home screen Cf/Org/Space/App cards
  • cards need to clip long text
    image
    • app-multiline-title can be used to avoid card dimension disparity
  • the text/type filters should be sticky for a session
  • Minor - Flesh out entity card actions
  • Cards show focus, needs uniformity with all other cards

Home Page - Recent

  • Recent List can be confusing
    • for instance
      • Two cf's in recent list (cf 1 at top and cf 2 at bottom)
      • Visit an app from cf 2 via the favourites list
      • Click on home button
      • Incorrect order shown (cf 1, app from cf 2, cf 2)
  • Feels like we should call this 'Frequent' to remove any feeling of 'last visited at top'
  • Entity titles in recent card should be clipped & match width set for recent section
    image
  • Most Recent list should be per console user. At the moment logging in as a different user shows same list - app-multiline-title can hopefully be used here too
  • Minor - Option to clear list of all recent entities
  • Minor - Option to remove a single item from recent list
  • App name (maybe others?) don't update on name change (either if changed in same session, or concurrently in another session and reload)
  • Entities in a disconnected endpoint are still shown in recents list (maybe these should just be hidden?)
  • Entities in a unregistered endpoint are still shown in recents list (think these should be removed permanently)

Home Page

  • Should wrap recent to below favourites on horizontal shrinkage
  • Minor - Would be super useful to have the links on the home page as anchors, such that the 'open in new tab' feature works. Feels like this page needs it most due to the potential number of entities that can be visited

Copy link
Contributor

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

I've updated the review comment above, plus new items below

History

  • new top bar icon means register an endpoint arrow points at wrong place (on endpoint page when no endpoints registered)
  • History card titles need to be clipped using app-multiline-title is possible
  • Minor - Choice of session tab should be sticky

Home Page

  • Personal opinion, labels would read a bit nicer as Top --> Frequent & History --> Recent

@KlapTrap
Copy link
Contributor Author

KlapTrap commented Feb 7, 2019

I've taken inspiration for the word "top" from firefox, that's what they use when showing you recent/frequently viewed links on the new tab screen.

I don't think clipping is needed, we have all the vertical space we need. I did try app-multiline-title on the favorite cards but it doesn't give good results when the height of the text is different to the meta card standard size.

I'm going to create another ticket with future work, this will include saving state across page views.

Nathan Jones and others added 12 commits February 7, 2019 12:41
* v2-master: (29 commits)
  Stop paginationResetPagination from added invalid fields
  Fix endpoint registration SSL error test
  Make sure we clear junk cookie to avoid issues
  Fix lint issue
  Fix for logging session secret detection
  Ensure we have same error output as V2
  Add logging for session store secret
  Fixed typos. Simplified
  Backend: Update to latest version of Echo Web Server (#3216)
  Fixes for SESSION_STORE_SECRET
  Tidy ups
  Fix header copy paste typo
  Remove spurious logging
  Add logging when upgrade volume detected
  Fix session secret with multiple instances. Fix multiple sessions
  WIP
  Updates for 8 Feb
  Update status_updates.md
  Updates
  Patch the image version in the chart, so reuse-values can work
  ...
- Removed commented out code
- Ensure we handle entities with loooooong names
- Wrap recent section on thin browsers
- Remove favourite actions if endpoint is disconnected
- Replaced `PaginationRemoveIdAction` with existing `ClearPaginationOfEntity`
Copy link
Contributor

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

LGTM, once gates pass will merge

Below is a summary of the follow on work

Home Page - Favourites

  • add favourites to services
  • add favourites to k8s
  • add type icons to favourites cards
  • Minor - Flesh out entity card actions
  • the text/type filters should be sticky for a session

Home Page - Recent

  • persist user top/recent lists over refresh
  • Minor - Option to clear list of all recent entities
  • Minor - Option to remove a single item from recent list

Home Page

  • Minor - Would be super useful to have the links on the home page as anchors, such that the 'open in new tab' feature works. Feels like this page needs it most due to the potential number of entities that can be visited

@richard-cox richard-cox merged commit 96af329 into v2-master Feb 15, 2019
@richard-cox richard-cox deleted the user-favorites branch February 15, 2019 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants