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

Fix top and bottom shadow of columns #357

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

Arif-Khalid
Copy link
Contributor

@Arif-Khalid Arif-Khalid commented Apr 4, 2024

Summary:

Fixes #354
Fixes #353

Type of change:

  • 🐛 Bug Fix

Changes Made:

  • Change CSS style for bottom shadow to use background image of scroll container
  • Refactor shadow style into its own class
  • Change background of child elements of scroll to transparent to be able to see background shadow
  • Reflect new reference for css shadows

Screenshots:

Before the change
image
image
After the change
image
image

Proposed Commit Message:

A few bugs exist related to column shadows. 
Top shadow is shown when there are no elements
behind header and hidden when there are.
Bottom shadow sticks to the column as a user scrolls.

These are not the intended behaviour of the shadows
to indicate presence of elements behind above or
below columns respectively.

Let's update the CSS to correspond to
appropriate shadow behaviours.

Checklist:

  • I have tested my changes thoroughly.
  • I have created tests for any new code files created in this PR or provided a link to a issue/PR that addresses this.
  • I have added or modified code comments to improve code readability where necessary.
  • I have updated the project's documentation as necessary.

@damithc
Copy link
Contributor

damithc commented Apr 4, 2024

@Arif-Khalid This is clearly a bug. So, let's write the commit message from that perspective. The current one sounds like we are tweaking the design, not fixing a bug.
Also, refactoring should not change behavior. So, this should not be called a refactoring.

Example:

Prevent bottom shadow from sticking to column
---------------------------------------------------
The shadow at the bottom of a column sticks to the column content
as the column is scrolled.

Let's fix this bug so that the shadow stays in place during scrolling.

@Arif-Khalid
Copy link
Contributor Author

@Arif-Khalid This is clearly a bug. So, let's write the commit message from that perspective. The current one sounds like we are tweaking the design, not fixing a bug. Also, refactoring should not change behavior. So, this should not be called a refactoring.

Example:

Prevent bottom shadow from sticking to column
---------------------------------------------------
The shadow at the bottom of a column sticks to the column content
as the column is scrolled.

Let's fix this bug so that the shadow stays in place during scrolling.

Understood. I have updated my PRs appropriately.

Copy link
Contributor

@NereusWB922 NereusWB922 left a comment

Choose a reason for hiding this comment

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

Bottom shadow should be hidden when reach the end of the list.

@Arif-Khalid Arif-Khalid changed the title Fix bottom shadow of columns Fix top and bottom shadow of columns Apr 8, 2024
@Arif-Khalid Arif-Khalid mentioned this pull request Apr 8, 2024
4 tasks
Copy link
Contributor

@NereusWB922 NereusWB922 left a comment

Choose a reason for hiding this comment

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

LGTM!

@Arif-Khalid Arif-Khalid merged commit 5ff13fe into CATcher-org:main Apr 10, 2024
3 checks passed
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.

Bottom edge shadow in the wrong position Top shadow hiding is incorrect
3 participants