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

new sparkbar examples and changes to allow bar only highlights #45

Merged
merged 3 commits into from Oct 13, 2023

Conversation

mattlangeman
Copy link
Contributor

Here are some Sparkbar examples. I ran into the situation of the current "area" Highlights not working well for Sparkbars, so I added "bar" highlights. Open to feedback on how this was implemented.

@vercel
Copy link

vercel bot commented Sep 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
layerchart ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 25, 2023 9:15pm

@what-the-diff
Copy link

what-the-diff bot commented Sep 25, 2023

PR Summary

  • Introduction of Dimension Getter in Highlight Component
    Firstly, a provision for a function named 'createDimensionGetter' is inserted into the Highlight component to potentially enable improved dimension management within the application.

  • Inclusion of 'bar' Property in Highlight Component
    The Highlight component now also has an 'export let bar' property along with associated parameters, enhancing its capability and feature set.

  • Addition of 'Sparkbar' to Navigation Menu
    A new menu item, 'Sparkbar', has been added to the application's main navigation, which will provide ease of access to the associated feature for end users.

  • Creation of Example Page for 'Sparkbar'
    To assist users in understanding and utilizing the new 'Sparkbar' feature, a demonstration page has been created, complete with illustrative code blocks and a visual chart component.

  • Page Source Import Approach in 'Sparkbar' Example
    The 'pageSource' import method has been incorporated on the 'Sparkbar' example page. This addition will ensure that the related raw data can be appropriately imported and displayed for reference.

  • Load Function Addition in 'Sparkbar' Example
    A new 'load' function has been added to the 'Sparkbar' example page. This will allow for more efficient loading of necessary components and data when accessing this page.

The above PR updated various segments of the code primarily to incorporate the 'Sparkbar' feature effectively into the application. These changes should contribute to a more robust and user-friendly interface overall.

@techniq
Copy link
Owner

techniq commented Sep 26, 2023

@mattlangeman I'm not against adding a new Highlight option, although another approach we could use it access the tooltip using let:tooltip and style the individual Bar using getProps (not overly sold on this name and might change it later).

CleanShot.2023-09-25.at.21.32.03.mp4

I used this approach to highlight some multi-line (and area) examples. See this post and follow up replies for a few examples.

With that said, I'm not against adding the tweening bar/rect shape. We could maybe support this using <Highlight area={{ snapToDataY: true }} > similar to TooltipContext.

Thoughts?

@mattlangeman
Copy link
Contributor Author

I actually first started looking into just changing the color of the bar directly. I didn't like duplicating the padding, stroke and radius. But I didn't come up with the getProps approach and thought I would need to then bring the tooltip into the bar component, which also didn't seem great.

You're thinking snapToDataY would replace the need for the extra bar prop to Highlight? Would we then also need snapToDataX for horizontal bar charts? I wonder if there is another name for this that would make sense.

Is there an example of the snapToData functionality somewhere? I found this note in the changelog, but it looks like at one point svelte-ux and layerchart were maybe in the same repo and now the release and commits have changed?

  • [Tooltip] Add "Snap to data" options to examples (09a92a)

@techniq
Copy link
Owner

techniq commented Sep 27, 2023

I actually first started looking into just changing the color of the bar directly. I didn't like duplicating the padding, stroke and radius. But I didn't come up with the getProps approach and thought I would need to then bring the tooltip into the bar component, which also didn't seem great.

You're thinking snapToDataY would replace the need for the extra bar prop to Highlight? Would we then also need snapToDataX for horizontal bar charts? I wonder if there is another name for this that would make sense.

That's what I was thinking, just to keep from having 2 props (area and bar) that were similar, and yes, there would be the compliment snapToDataY. I'm open for another name, or inverting the meaning and having the full band width/height me the option. There might also be a more elegant solution. I'm also not completely against adding bar if it makes the most sense, just feeling out different approaches.

Is there an example of the snapToData functionality somewhere? I found this note in the changelog, but it looks like at one point svelte-ux and layerchart were maybe in the same repo and now the release and commits have changed?

  • [Tooltip] Add "Snap to data" options to examples (09a92a)

I think that might be an overlook when I generated the initial changelog from commits (before I started using changesets). If you notice, the actual commit is in the layerchart repo.

Examples of the prop are available on the Tooltip component page, with the Scatter example having it enabled by default.

image page

@techniq
Copy link
Owner

techniq commented Sep 27, 2023

Another thought I had was to use a clipPath to "mask" the <Highlight area={...} /> so it makes the same shape of the bars themselves, but I tried a few quick examples and wasn't having any success. It would provide a slightly different effect to the spring/tweened shape <Highlight bar> provides.

I'm fine with leaving this PR as is for now, unless you want to explore some more. I wish it didn't result in some duplication/added complexity within Highlight, but maybe we circle back on it later.

We could also extract a Bar component that is used within Bars and Highlight.

I'm thinking we could also remove the getProps and just expose a bar slot with slot props passed.

I have to say createDimensionGetter is rather ugly, but it was the best solution I came up with at the time.

@mattlangeman
Copy link
Contributor Author

Examples of the prop are available on the Tooltip component page

Ah. I see now. I was looking for code within the examples directory instead of across all docs. So snapToData in these examples is to fix the position of the Tooltip instead of having it move a bit based on cursor. That makes sense to me. I'd lean towards keeping the terminology different bar highlighting. The other thing I was thinking this morning is that it would be nice to allow both current area highlights and bar highlights at the same time. This could still be accomplished with only the area prop. Something like area={'band'=true, 'bar'=true}. And defaults could be band=true if backwards compatibility is desired.

I'm not in a hurry to get this PR merged in and wouldn't mind exploring a bit more. I have a busy day today, but plan to revisit tomorrow.

@techniq
Copy link
Owner

techniq commented Sep 27, 2023

@mattlangeman Sounds great, and I'm not in a rush either :). I'd rather refine and iterate, with the best abstraction to support the most use cases, with simplest implementation to simplify maintenance / additional use cases (you know, the holy grail).

Examples of the prop are available on the Tooltip component page

Ah. I see now. I was looking for code within the examples directory instead of across all docs. So snapToData in these examples is to fix the position of the Tooltip instead of having it move a bit based on cursor. That makes sense to me. I'd lean towards keeping the terminology different bar highlighting.

With TooltipContext / Tooltip, there are basically 3 ways to position:

  • By default, TooltipContext will use the mouse/pointer position, and Tooltip will apply an offset by default (mostly to not cover up the point you are interested in)
  • In TooltipContext, you can opt into snapping the position to the closest data point's X and/or Y
  • In Tooltip, you can specify explicit top / left positions, overwriting/ignoring what is set in the TooltipContext. This was a recent addition we made for the Sparkline example, but I'd like to add more examples to show what kind of use cases this supports
    • A fixed tooltip in the top left (or any corner) by specifying values for both props (ex. <Tooltip top={8} left={8} />)
    • Only specifying a fixed top such as this visx example or these Observable plot examples which would use 2 instances of <Tooltip>, one with a fixed top and one with a fixed left.

The other thing I was thinking this morning is that it would be nice to allow both current area highlights and bar highlights at the same time. This could still be accomplished with only the area prop. Something like area={'band'=true, 'bar'=true}. And defaults could be band=true if backwards compatibility is desired.

I'm not in a hurry to get this PR merged in and wouldn't mind exploring a bit more. I have a busy day today, but plan to revisit tomorrow.

One thing to note, is you can have multiple instances of both <Highlight> and <Tooltip>. One example for Highlight is the PunchCard example. So we could also do <Highlight area={{ mode: 'band' }}> and also <Highlight area={{ mode: 'bar' }}>, although I also try to support using a single component when possible (why you see <Highlight points lines /> used a lot to turn on both points and lines).

Also, the order you order the components matters (last one is on top, as it's svg). So...

<Points>
<Highlight area />

...would put the Highlight on top of the the Points (and why it's 5% opaque by default). While...

<Highlight area />
<Points>

would put the Highlight behind the Points. In some ways this is more aesthetically pleasing (but just depends on the general chart).

Sorry, this was as longer reply than I intended :). One focus I hope to accomplishing before tagging the 1.0 release is to punch up the documentation a lot, trying to document and describe a lot of this "hidden" functionality/capability. I'm also hoping as more users are using the library, it will naturally improve the docs.

Lastly, if you'd ever want to jump on a video chat to discuss anything, I'd be open for it.

@mattlangeman
Copy link
Contributor Author

I was playing around with the bar highlighting again today. I tried sending in the tooltip to the Bar component so that it could be used to create the highlight within Bar. That allowed for creating the tweened bar, but without duplication of radius, stroke, etc. However, I realized that you may actually want to change the stroke for the highlight bar, and that led me back to thinking it may be best to create the rect within Highlight with the different modes.

The other idea was pulling in the context of Bar into Highlight via setContext and getContent and using that as defaults when creating the rect in Highlight.

It would be great to have a video call sometime. My schedule can be sporadic, but has also has some flexibility. Usually my ideal times are between 10am-4pm Eastern or in the evening after 8:30pm.

One topic I'm curious about is dealing with missing data and multiple datasets/different sources of data. A recent project had a unique challenges around those areas.

@techniq
Copy link
Owner

techniq commented Sep 28, 2023

Evenings typically work best for me, as I too have a sporadic schedule (between work and 3 young kids/activities). I've setup a discord server if you want to chat (invite link), and we can figure out the details when/how to do a video chat (google meet, zoom, teams, etc). I'm actually off tomorrow and could chat during the day if you'd prefer. Just let me know.

@techniq techniq merged commit e0d65ba into techniq:master Oct 13, 2023
2 checks passed
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.

None yet

2 participants