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

#1387 - List Page: allow the list table to horizontally scrolled #2364

Merged
merged 8 commits into from
Feb 14, 2024

Conversation

EthanW96
Copy link
Collaborator

@EthanW96 EthanW96 commented Feb 8, 2024

I tried adding

 display: block;
overflow-x: auto;
white-space: nowrap;

to table.js-list and that seems to be a possible solution to this issue.

I also tried to implement sticky columns by adding a data-id to (for css targeting) and adding some css to target the "name" column if it exists on a table. I think this leads to some questions.

  • Are we wanting users to select which columns can be made sticky?
  • Is there any issue with adding the data-id attribute to the elements in a table? The elements already had them so I figured this was a reasonable addition.

@corsacca
Copy link
Member

Thanks Ethan!
This is a simpler solution than converting to using the components version :).
I like the sticky name column.

Are we wanting users to select which columns can be made sticky?

This seem like it would complicate the UI a bit. Maybe something we'd add in an upgrade with any other feedback.

Is there any issue with adding the data-id attribute to the elements in a table?

Nope, great add.

Comments:

  • Needing clues that more fields are hidden just off screen. Maybe having the scrool bar be at the top and the bottom?

image

  • With few fields the columns don't use all the space. Maybe not an issue.

image

  • Mobile view the field names use more space. (mobile view needs some attention anyways)
    image

This also could help on larger screens: #2370

@EthanW96
Copy link
Collaborator Author

Awesome! Glad this is simpler for now. I've added another commit to fix the first two comments:

Needing clues that more fields are hidden just off screen. Maybe having the scrool bar be at the top and the bottom?

image

With few fields the columns don't use all the space. Maybe not an issue.

image

The last comment I'm either not clear on the issue or I can't recreate it. Can you walk me through that question?

@corsacca
Copy link
Member

Very nice!

I added a commit to fix the mobile issue.
This shows all the email values instead of them being hidden to the right:
image

@corsacca
Copy link
Member

Can we keep the highlight color on the name cell during hover?
image

@EthanW96
Copy link
Collaborator Author

@corsacca I think the highlight color comment has been resolved with this most recent commit.

Can we keep the highlight color on the name cell during hover?

@corsacca
Copy link
Member

@EthanW96, I showed @ahillbilly. He loves it too!
Can we keep the number sticky too?
And can we have a max width for the column to keep a fields from being really long?
image

@EthanW96
Copy link
Collaborator Author

@corsacca

Can we keep the number sticky too?

Yes certainly! I set the left: 0 to left: -1px after I noticed on the extreme left edge of the table I could see the underlying text in a 1px width gap. Not sure how that happened or if that's just me.

And can we have a max width for the column to keep a fields from being really long?

I experimented a bit and 250px seemed like a good middle ground. Thoughts?

@corsacca
Copy link
Member

Nice work! 🎉 🎉 🎉
image

@EthanW96
Copy link
Collaborator Author

Good call on the max-width for mobile view 👍 didn't think about that. Thanks for your help @corsacca!

@corsacca corsacca merged commit 3ebdc46 into DiscipleTools:develop Feb 14, 2024
2 checks passed
@corsacca corsacca linked an issue Feb 14, 2024 that may be closed by this pull request
@corsacca
Copy link
Member

Thank you @EthanW96!

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.

List Page: allow the list table to horizontally scrolled
2 participants