Feature/3331 dynamic column width
- Resolves: #3331 + #6457
- Target version: main
Summary
Following the descriptions in the linked ticket, I have updated the styles for the board, overview, stack and search results cards, so that they behave more responsive according to the space available. As a side-effect it also fixed issue #6457.
On top of that, this PR contains the following changes, which aren't necessarily part of the issue, but I thought they'd fit in:
- aligned design for board and overview
- aligned spacing inside stack between header, cards and new card form
- refactored how border radius is achieved on card image
- added v-for loop in overview to get rid of code duplication
- fixed border around add list
- used
existingIndexto defineexistingCardto avoid extra lookup - fixed binding of title attibute on stack column title for boards
- improved accessibility attributes on column for overview
- changed title phrasing of search results depending on view
| new | old |
|---|---|
TODO
N/A
Checklist
- [ ] Code is properly formatted
- [ ] Sign-off message is added to all commits
- [ ] Tests (unit, integration, api and/or acceptance) are included
- [ ] Documentation (manuals or wiki) has been updated or is not required
Thanks a lot for your contribution. This looks quite good. I'd love to collect some feedback from designers like @marcoambrosini on that.
The code changes look sane to me, also some nice refactorings along the way. I'll give this a test run later. Can you maybe address the stylelint and eslint errors that CI reported?
Is the change of the scroll container intentional? I'm personally not a fan of scrolling the full board area and would rather prefer to stick with the per column scrolling we used to have before.
Just noticed the merge commit, could you do a rebase to the main branch instead so we have a clean git history of your changes?
Thanks for your review! I've just added a commit to fix the linting issue. Regarding the scrolling, my change is intentional, but of course I can add an new commit so that scrolling is inside the column again. I should have time during the weekend to do that. I'll do a rebase too!
Regarding the scrolling, my change is intentional, but of course I can add an new commit so that scrolling is inside the column again. I should have time during the weekend to do that. I'll do a rebase too!
That would be great, thanks a lot already. :)
Hi @ludij, I just had a quick look at our competitors and it seems that varying the with of the columns is not a very common practice. These columns are usually fixed in width and horizontal scrolling is used for overflowing lists. @juliusknorr what do you think about that? That said, in principle this seems like a sane change to me design-wise, I would just make sure to the min and max values not too far apart from each other to not completely change appearance because that could be disorienting. I need some time to setup my development environment again to properly test it. I'll come back for a review tomorrow. Thanks a lot for for picking this up :)
Hi @ludij, I just had a quick look at our competitors and it seems that varying the with of the columns is not a very common practice. These columns are usually fixed in width and horizontal scrolling is used for overflowing lists. @juliusknorr what do you think about that?
It's pretty easy to update this code so that the column width is fixed. Imho my refactor probably makes sense either way, because it allows the widths and spacings for the different parts to be maintained from a single source (variables.scss (at least that's what I aimed for :) )), whereas before it was needed to adjust multiple files and lines.
That said, in principle this seems like a sane change to me design-wise, I would just make sure to the min and max values not too far apart from each other to not completely change appearance because that could be disorienting. I need some time to setup my development environment again to properly test it. I'll come back for a review tomorrow.
Sounds good. I'm not so certain whether my changes to the spacing inside the card look good (with/without labels, with/without image, etc.). I also just noticed that the spacing inside the "Add new card" card doesn't align perfectly with the header and other cards in the column. I could easily fix that for the "Add new card", but it'd be great to get some feedback on the updated spacing inside the cards in general before, so I can do an update in one go.
Thanks a lot for for picking this up :)
Very happy to contribute and thanks for the feedback!
Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.
We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.
Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6
Thank you for contributing to Nextcloud and we hope to hear from you soon!
(If you believe you should not receive this message, you can add yourself to the blocklist.)
@ludij Can you rebase your pull request to the latest main branch to resolve the conflict?
@juliusknorr + @grnd-alt Thanks for the feedback and sorry for the delay. I'm not very familiar with the rebase workflow, so it took me a few pushes, but I think everything should be fine now 🚀
@luka-nextcloud what's preventing this from being merged?