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

Display tags on each document on the course view #295

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

Mortinat
Copy link
Contributor

@Mortinat Mortinat commented Jan 2, 2023

image

They are bit small but their color make them recognizable. We could maybe think to icon related to each filter to help the visualisation

@C4ptainCrunch
Copy link
Contributor

For context, see #251

What does it look like on mobile ? Especially with a lot of tags on the same document ?
Personally, i'm not a fan of the colors and the fact that it clutters the UI a lot. That's why i hid them in the first place.
Does somebody have an idea to make them look better ?

@C4ptainCrunch C4ptainCrunch changed the title Display the tags Display tags on each document on the course view Jan 3, 2023
@Mortinat
Copy link
Contributor Author

Mortinat commented Jan 3, 2023

image
look like this on an "Iphone XR"
image
this on an smaller screen "Iphone SE"

And I don't think any documents will have more tags that what I added to modsim

@Mortinat
Copy link
Contributor Author

Mortinat commented Jan 3, 2023

For the color I think we should choose color more distinct to each other, and as I propose maybe just have an icon related to a filter and just display the icon with the right color cool simplify the ui

@C4ptainCrunch
Copy link
Contributor

C4ptainCrunch commented Jan 4, 2023

As maintainer, i'm against the color as they are now, it's too distracting and has no intrinsic meaning, without any clear benefit. If we want to go further with this PR, we should find a better way to color (or maybe even not color) the tags.

But i like the layout and that if works well with a line break on small screens 🚀

just have an icon related to a filter and just display the icon with the right color cool simplify the ui

I don't understand the phrase, could you maybe do a mockup of what you are envisioning ?

My feeling is this PR is not mature enough as we haven't even decided what the UI should look like yet. It merits a longer discussion first. I'll move this to a draft.

Edit: i know this PR was not a lot of work so this edit might not really apply but...

For a subject is know to need a discussion, like this one where a ticket (stating the color/UI clutter problems) is already open, cf #251), it's better to first discuss, make mockups and agree and then implement.
So that we don't arrive in a place where some code that was already written and has to be heavily modified of even thrown out because it does not match what as been decided :)

@C4ptainCrunch C4ptainCrunch marked this pull request as draft January 4, 2023 12:49
@Mortinat
Copy link
Contributor Author

Mortinat commented Jan 4, 2023

I don't really get why it becomes an issue now, before the tags as been always showed like that. Actually there is no way to distinct the different type of documents easily, so maybe it's not the perfect solution but it's a good work-a-round until we find a better solution.
In the form we did a year ago the people find that tag was one of the greatest point and we just "removed" it (we can filter but it's not obvious as it was before)

@C4ptainCrunch
Copy link
Contributor

In the form we did a year ago the people find that tag was one of the greatest point

This is the kind of information that would have been great to point in the original ticket (#251). If it's the case, this clearly increases the priority of this ticket. Could you share the results of the form somewhere (another issue just for that would be great :) )

tags as been always showed like that

We always have done like that is not a valid reason to keep going.

Actually there is no way to distinct the different type of documents easily

I do get that you want to show the tags. I do too !
But the decision i took during the redesign. And i still stand by it.
(That's also why the tags where behind a display: none flag that you only had to remove to show them in the PR)

It is just that showing them with bright colored pills is worse than not showing them.

We, as developpers have a tendency to want to display everything, every option, everywhere. But thats how we end with cramped user interfaces impossible to learn for a new, non computer savy user.

UI/UX is difficult (especially for developpers) and we have to say "no" to adding things sometimes. This is suboptimal for the feature we want, but is for a greater good of the project in itself.


So again, let's first think on how to make this feature look like, let's make mockups and when we decide it's better than the status quo we can implement :)

@Mortinat
Copy link
Contributor Author

Mortinat commented Jan 5, 2023

#297 I put the link there

For me the color pills are great but maybe not with the text, having a color associate with a type of documents make it really easy to spot what you're searching. I will try to make a version with the icon idea I had in the next days.

@Mortinat
Copy link
Contributor Author

Mortinat commented May 31, 2023

@C4ptainCrunch what about this
image
image

@Mortinat
Copy link
Contributor Author

if we agree on that I will do some css for add a break on mobile

@Mortinat Mortinat marked this pull request as ready for review May 31, 2023 15:22
@C4ptainCrunch
Copy link
Contributor

C4ptainCrunch commented Jun 1, 2023

I'm not a fan of this solution either, for multiple reasons:

  • Colors are very distracting, they attract all the attention, even more than the title of the document
  • The icons are confusing (eg. a pencil ? is this a button to edit the document ?)
  • The pills are way bigger in height than the rest of the line, it breaks the layout

What about trying a mockup with only text and see if it's legible ?

@Mortinat
Copy link
Contributor Author

Mortinat commented Jun 1, 2023

Sorry I forgot to mention the icon I chose was just to show what it could look like, not definitive (they are modifiable from the admin).
For me the goal is to be able to quickly distinct which type of document they are and not start to read each tag, so in my sens color are mandatory.Colors are very distracting, they attract all the attention, even more than the title of the document actually in many case the title is not the main information or gave the same info as the tags example :
image
It also happens that title doesn't give the information of what the document is but the tags does
image
here all the documents are slides but the title doesn't give this info.
(all the example are based on the course https://dochub.be/catalog/course/info-f102)

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.

Les tags ne sont pas visibles sur la liste des documents d'un cours
2 participants