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

MBS-8586: Simplify horizontal scrolling #34

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

sh-csg
Copy link
Contributor

@sh-csg sh-csg commented Jan 7, 2024

No description provided.

@sh-csg sh-csg self-assigned this Jan 7, 2024
@sh-csg sh-csg linked an issue Jan 7, 2024 that may be closed by this pull request
Copy link
Member

@PhMemmel PhMemmel left a comment

Choose a reason for hiding this comment

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

Looks good in general.

Some minor questions/thoughts/suggestions:

  • Would it be a good idea to make the steps when clicking the buttons dependent on the card with? Currently it feels a bit like I have to click too often to move right, so maybe we can use cardwith/2 as scroll length or even 0.9*cardwith or something?
  • To fit into the overall design maybe we could style the buttons with rounded corners.
  • It would be nice if the buttons could be transparent, so when they are shown, you can still see the content of the card behind them
  • User might eventually expect that I can keep scrolling by keeping the buttons pressed. Is this something we want?

styles.css Outdated

.mod_kanban_scroll_right {
left: calc(100vw - 30px);
}
Copy link
Member

Choose a reason for hiding this comment

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

missing new line

Copy link
Member

Choose a reason for hiding this comment

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

still no newline

@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch from 2a223fe to 7b396a6 Compare January 18, 2024 14:20
@sh-csg
Copy link
Contributor Author

sh-csg commented Jan 18, 2024

Introducing transparency produces a quite messy output - maybe we'll find another approach for implementing
this.
Default scrolling distance is now the width of a column, inner corners are rounded.
Whether to keep scrolling when button is constantly pressed is an open question for me - I would move this to another ticket,

@sh-csg sh-csg requested a review from PhMemmel January 24, 2024 13:14
@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch from 7b396a6 to 0e33f83 Compare January 25, 2024 06:39
@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch from 0e33f83 to 5506e84 Compare January 25, 2024 06:47
amd/src/board.js Outdated
} else {
this.getElement(selectors.SCROLLLEFT).style.setProperty('visibility', 'visible');
}
if (main.scrollLeft < main.scrollLeftMax) {
Copy link
Member

Choose a reason for hiding this comment

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

Not a standard property, does not work on Chrome and many other browsers, see: https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollLeftMax?retiredLocale=de

Needs to be changed.

Copy link
Member

@PhMemmel PhMemmel left a comment

Choose a reason for hiding this comment

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

Pls see comments.

@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch from 5506e84 to 48b3ebe Compare February 1, 2024 18:52
@sh-csg
Copy link
Contributor Author

sh-csg commented Feb 1, 2024

Accidentally pushed wrong version - sorry for that!

@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch from 48b3ebe to 34a7e55 Compare February 3, 2024 14:53
@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch 5 times, most recently from 386a094 to 8a80298 Compare February 13, 2024 10:34
@sh-csg sh-csg closed this Feb 13, 2024
@sh-csg sh-csg force-pushed the MBS-8586-Simplify-horizontal-scrolling branch from 8a80298 to 9b283d9 Compare February 13, 2024 10:34
@sh-csg sh-csg reopened this Feb 13, 2024
Copy link
Member

@PhMemmel PhMemmel left a comment

Choose a reason for hiding this comment

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

Great feature, well done!

Internally reviewed, happy to merge.

Some more improvements regarding button styling and general handling will be introduced in a separate PR.

@PhMemmel PhMemmel merged commit 3a975bb into master Feb 13, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Horizontal scrolling
2 participants