Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Feature/15.0/19853/_add_report_description #4

Closed

Conversation

Jessica-BlueStingray
Copy link

Add report description [19853]

A description area was added to the Control Panel between the View name and the Search Area.
To test:

  1. Create a fresh db
  2. Install Sales (but can be tested in other modules too)
  3. Turn on debug mode
  4. Click on the debug button > Edit Action > Add a text to view_description > Save > Refresh page
view_description.mp4

If it doesn't show up at first, refresh the browser (Ctrl+R) one or more times, going to another view and coming back to the previous view also works..

Copy link

@dpsholtes dpsholtes left a comment

Choose a reason for hiding this comment

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

Untested on this pass. Just a couple questions and nitpicks

@traviswaelbro
Copy link

@Jessica-BlueStingray Let's keep this moving 🙏

Copy link

@dpsholtes dpsholtes left a comment

Choose a reason for hiding this comment

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

Functionally tested, looking good overall 👍

Just one bit of commented out code to omit.

My only concern is this part of the requirements:

Responsive design reclaims that whitespace for smaller screen widths, let’s hide the description as necessary.

I tested this out, and it does not appear to hide, and sometimes pushes the search bar off of the viewing area.

addons/web/static/src/legacy/js/chrome/abstract_action.js Outdated Show resolved Hide resolved
Copy link

@dpsholtes dpsholtes left a comment

Choose a reason for hiding this comment

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

Not retested. Looking good 👍

Copy link

@traviswaelbro traviswaelbro left a comment

Choose a reason for hiding this comment

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

Not retested but I did test previously ✅

@Jessica-BlueStingray
Copy link
Author

Jessica-BlueStingray commented Sep 2, 2022

@traviswaelbro @dpsholtes @toddythebody This PR was rebased and is ready.

Copy link

@dpsholtes dpsholtes left a comment

Choose a reason for hiding this comment

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

Not retested

@traviswaelbro
Copy link

@Jessica-BlueStingray Leaving this for you to merge as you see fit

@Jessica-BlueStingray
Copy link
Author

Jessica-BlueStingray commented Sep 21, 2022

Waiting on Adam to see if the other PR with this code is going to be merged there or not. Still not really sure on what is the next move there.

@traviswaelbro
Copy link

Are you referring to metricwise#20, where there was some discussion about sed commands?

@Jessica-BlueStingray Jessica-BlueStingray force-pushed the feature/15.0/19853/_add_report_description branch from 3ebff71 to e9fa865 Compare September 22, 2022 21:37
@Jessica-BlueStingray Jessica-BlueStingray force-pushed the feature/15.0/19853/_add_report_description branch from e9fa865 to 9c52041 Compare September 22, 2022 21:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants