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

Auditorium UI: Emphasise weekends better #480

Closed
hendr-ik opened this issue Sep 20, 2020 · 13 comments
Closed

Auditorium UI: Emphasise weekends better #480

hendr-ik opened this issue Sep 20, 2020 · 13 comments

Comments

@hendr-ik
Copy link
Member

This is the way we currently emphasise weekends in our Plotly bar charts - simple numbers, only slightly larger, italicized:
bars is

We consider this not obvious enough and woud like to update it this way:
bars want

Ideally it should be implemented directly in Plotly without extra css. The relevant component is this one:
https://github.com/offen/offen/blob/master/auditorium/src/views/components/auditorium/chart.js

@hendr-ik hendr-ik added Hacktoberfest help wanted Extra attention is needed labels Sep 20, 2020
@haridevelops
Copy link

Can I work on it ? If so please assign it to me.

@m90 m90 changed the title Auditorium UI: Emphasis weekends better Auditorium UI: Emphasise weekends better Sep 20, 2020
@hendr-ik hendr-ik assigned hendr-ik and haridevelops and unassigned hendr-ik Oct 1, 2020
@hendr-ik
Copy link
Member Author

hendr-ik commented Oct 1, 2020

@haridevelops let me know if there are any questions

@prijeshb
Copy link

prijeshb commented Oct 7, 2020

@hendr-ik Should I take this, seems that no progress further by other person.

@m90
Copy link
Member

m90 commented Oct 7, 2020

Thanks @prijeshb , let's maybe check with @haridevelops if they still plan to work on this? Any input from your side?

@prijeshb
Copy link

prijeshb commented Oct 7, 2020

@hendr-ik Should I take this, seems that no progress further by other person.

Also, looked documentation but didn't found any relevant attribute to set background color for ticks

@m90
Copy link
Member

m90 commented Oct 8, 2020

@prijeshb yeah, this will require some creative use of Plotly (negative bars maybe?) and is not something that is supported out of the box as-is. Maybe there is some other solution we don't know of yet?

Let me know if you want to work on this in any case so I can assign you. Thanks.

@haridevelops
Copy link

@hendr-ik
I did look at the plotly documentation, we can provide bgcolor for all the ticks but again we need to go through all the logic that has ran for ticktext.
The way chart.js weekend logic is written, simple css fix would work right? (the font-style: italic is handled inline css only, we can replace with background-color css.)

@m90
Copy link
Member

m90 commented Oct 10, 2020

@haridevelops When I try applying a background-color to the span here:

if (isWeekend(date)) {
return `<span style="font-style: italic; color: ${tickColorFade}; font-size: 130%">${result}</span>`
}

it doesn't really display any background for me. Is there something needed in addition to that?

@haridevelops
Copy link

@m90 I have locally up the code.
Plot is basically rendering SVG. The problem here is tspan tag. After research, i realise that tspan tag does not support background color. Refer here: https://developer.mozilla.org/en-US/docs/Web/SVG/Element/tspan (there is no presentation attribute supporting background)

image

I would like to recommend different aspect here emphasising weekend. Thoughts?

image

@m90
Copy link
Member

m90 commented Oct 10, 2020

@haridevelops Thanks for doing that research. It's tricky business indeed.

I would like to recommend different aspect here emphasising weekend. Thoughts?

Thing is we need to / want to comply with WACG 2.1 Accessibility Guidelines here, so styling this accordingly is a complicated topic. @hendr-ik is on holidays for a few more days, but I would suggest he follows up on this with you once he's back. Does this sound like a plan?

@haridevelops
Copy link

perfect @m90

@hendr-ik
Copy link
Member Author

@haridevelops Thanks again for your efforts and the suggestion. Unfortunately, we cannot use green for highlighting because it is only used in connection with links in our UI. I think we need to dig deeper into Plotly and create a design that fits the possibilities.

@hendr-ik
Copy link
Member Author

@haridevelops Please do not hesitate to submit your recommendation as a pull request in order to be officially recognized. Unfortunately, for the reasons mentioned above, we will not be able to merge it.

@m90 m90 closed this as not planned Won't fix, can't repro, duplicate, stale May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants