Problem/Motivation

svelte-split-pane is used to create columns within the project browser. However, it's quite opinionated and does not offer flexible ways to adjust styles based on viewport width (i.e. stack on mobile). Flexbox + media queries should be sufficient.

Steps to reproduce

Proposed resolution

At the very least, reproduce the svelte-split-pane markup/CSS directly in the components using them. If these styles need to be applied to multiple components, they can go in a parent component with the global attribute (<style global>.your-class { styles...}).

I think it would be reasonable to also add some responsiveness within the scope of this issue, so the tiles stack at narrower widths. However, it'd be just as fine to defer that to a followup.

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Addition/Change/Update/Fix to this project
  • ☐ Testing to ensure no regression
  • ☐ Automated unit/functional testing coverage
  • ☐ Developer Documentation support on feature change/addition
  • ☐ User Guide Documentation support on feature change/addition
  • ☐ Code review from 1 Drupal core team member
  • ☐ Full testing and approval
  • ☐ Credit contributors
  • ☐ Review with the product owner
  • ☐ Release

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#21 Screenshot 2022-05-23 at 12.13.17.png21.24 KBfjgarlin

Issue fork project_browser-3280706

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bnjmnm created an issue. See original summary.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: Unassigned » hooroomoo

hooroomoo opened merge request !85

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Issue tags: +Project Browser MVP
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

Does Claro offer any classes which do this kind of flexible layout without us needing styles at all? Just wondering out loud. Ultimately my goal is to have as minimal CSS as possible in the Svelte components, and lean on Claro and classes to try and get there.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

Does Claro offer any classes which do this kind of flexible layout without us needing styles at all?

Unfortunately Claro doesn't have this, layout styling is prettyspecific to the item being styled.

Something that's not likely feasible for PB 1.0, but worth mentioning: Layout Builder has configurable column layouts that theoretically could take care of this (see core/modules/layout_builder/layouts/twocol_section/twocol_section.css). It may not even be necessary to use Layout Builder as Layout Discovery is independent of that (although the layout might need to be moved outside of LB). Even if it's feasible it's probably not a great idea to leverage it when designs are still changing - this could potentially add a bunch of overhead to making those changes.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Status: Active » Needs review
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented

The list card view got a little messed up after removing floats, so I created https://www.drupal.org/project/project_browser/issues/3281198 to address that. (Decided to just create a follow up issue since the design is going to change soon and it will need to be updated anyway).

Also created https://www.drupal.org/project/project_browser/issues/3281200 to address the categories filter taking up the entire screen on small screens.

bnjmnm opened merge request !98

hooroomoo closed merge request !85

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: hooroomoo » Unassigned
fjgarlin’s picture

fjgarlin commented
Assigned: Unassigned » fjgarlin
fjgarlin’s picture

fjgarlin commented
Assigned: fjgarlin » Unassigned
Status: Needs review » Reviewed & tested by the community

I've seen the issues but also saw that you created the follow-ups too. I think this is still an improvement when on desktop, and for mobile too, having as main issues the follow-ups.

The changes in the MR are pretty clear too, so marking as RTBC.

tim.plunkett made their first commit to this issue’s fork.

  • tim.plunkett committed 0414373 on 1.0.x authored by bnjmnm 
    Issue #3280706 by hooroomoo, bnjmnm: Remove svelte-split-pane, replace...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Fixed

Merged, thanks! One less dependency, +1
And thanks for opening the followups

  • tim.plunkett committed 228ed74 on 1.0.x 
    Revert "Issue #3280706 by hooroomoo, bnjmnm: Remove svelte-split-pane,...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Fixed » Needs work

I committed this after rebasing before waiting for tests to pass :facepalm:
Reverted this for now, it's interfering with one of the tests, testCategoryFiltering

fjgarlin opened merge request !105

fjgarlin’s picture

fjgarlin commented

I created a new branch with the exact same changes as before as the previous MR was actually merged.
I ran the tests locally and the test returns "25 Results" but expects "19 Results".

I don't understand how this changes might affect the results, but they do. I guess it's related to the fixture data and the mock query which I don't know too much about. Just wanted to put the above info and re-roll the MR to help with this, but I'm not sure how to debug the difference in results.

fjgarlin’s picture

fjgarlin commented
Assigned: Unassigned » hooroomoo
StatusFileSize
new Screenshot 2022-05-23 at 12.13.17.png21.24 KB

I managed to fix the tests (yay!) but I saw there is some styling differences with the unpatched versions around the "Grid / List" buttons, which is related to this MR, see screenshot.
Screenshot

That part "Needs work".

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Status: Needs work » Needs review
hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Assigned: hooroomoo » Unassigned
fjgarlin’s picture

fjgarlin commented
Assigned: Unassigned » fjgarlin

I'm testing this.

fjgarlin’s picture

fjgarlin commented
Status: Needs review » Reviewed & tested by the community

Checked code and tested it all and it looks good. RTBC.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: fjgarlin » Unassigned
Status: Reviewed & tested by the community » Fixed

Great, thanks for fixing. The new MR looks nice and clean.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.